clang-randstruct / llvm-project

Randomize the order of fields in a structure layout as a compile-time hardening feature
3 stars 1 forks source link

Add No Randomize Layout attribute to regression test #37

Closed tim-pugh closed 5 years ago

tim-pugh commented 5 years ago

Since we've merged in the new attribute we will need to add it to the regression test.

tim-pugh commented 5 years ago

Closing this issue. I just ran the regression test suite make clang-test and it passed without error.

This is rather odd considering we added the second attribute but we'll re-run it at a later date. It may be because it's not being used properly quite yet.

connorkuehl commented 5 years ago

This is rather odd considering we added the second attribute but we'll re-run it at a later date. It may be because it's not being used properly quite yet.

I was confused about this this morning too, but @Jafosterja mentioned this code was reverted.

See commits fadd2474bf7519494a9fe0b016d970baaf755f65 and f94de0689a0218ed06322b6207b854b332835321

Searching the codebase for "Randomize Layout" confirms this code is absent (since "No Randomize Layout" matches "Randomize Layout")

connor@mead-garden:~/src/git/llvm-project/clang$ ag -i randomizelayout
test/Misc/pragma-attribute-supported-attributes-list.test
120:// CHECK-NEXT: RandomizeLayout (SubjectMatchRule_record)

lib/AST/RecordLayoutBuilder.cpp
2991:  bool ShouldBeRandomized = D->getAttr<RandomizeLayoutAttr>() != nullptr;

lib/Sema/SemaDeclAttr.cpp
6856:  case ParsedAttr::AT_RandomizeLayout:
6857:    handleSimpleAttribute<RandomizeLayoutAttr>(S, D, AL);

include/clang/Basic/Attr.td
3205:def RandomizeLayout : InheritableAttr {
tim-pugh commented 5 years ago

https://github.com/clang-randstruct/llvm-project/pull/27/commits/f06d4cc861b166735a8b1e57faf287e4e7ea7c11

tim-pugh commented 5 years ago

We can pull those in now, but from what I'm seeing, they were removed. Does this make sense?

tim-pugh commented 5 years ago

https://github.com/clang-randstruct/llvm-project/commit/d95e6247f5ddd2812f03bd3a829d9299b751abfe

This was the revert commit

connorkuehl commented 5 years ago

This was the revert commit

Yeah, that's the source of my confusion. I saw that those commits I linked earlier were merged in to develop but I couldn't find where they were taken out.

connorkuehl commented 5 years ago

Closed by #39