Skip to content

Skip unions #33

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
Feb 27, 2019
Merged

Skip unions #33

merged 2 commits into from
Feb 27, 2019

Conversation

connorkuehl
Copy link

@connorkuehl connorkuehl commented Feb 26, 2019 •
edited
Loading

Forgot to bring this over when we originally ported from the plugin. Closes #23

With this PR, the Clang compiler will not randomize unions and it will emit a warning when someone decorates their union with the randomize_layout attribute.

/home/kuehlcon/llvm-project/build/bin/clang -g -Wno-format poc.c -o rand
poc.c:33:7: warning: union declared with 'randomize_layout' attribute [-Wrandomize-layout]
union u {
    ^

clang-test:

[100%] Running the Clang regression tests
llvm-lit: /home/kuehlcon/llvm-project/llvm/utils/lit/lit/llvm/config.py:337: note: using clang: /home/kuehlcon/llvm-project/build/bin/clang
Testing Time: 402.89s                                                                       
  Expected Passes    : 14089                                            
  Expected Failures  : 19                               
  Unsupported Tests  : 62
  [100%] Built target check-clang                     
  [100%] Built target clang-test

@connorkuehl
Skip Unions when selecting from randomization
fe25c0e 
There's no harmful effect if we DO randomize a union, however,
since all of a union's data members have their offsets set to 0
there's no advantage for our code to run anyway. This way we can
spare some CPU cycles in the event that someone adds the randomize
attribute to their union.
@connorkuehl
Add warning when union has randomize_layout attr
1731e3a
jcantrell
jcantrell approved these changes Feb 27, 2019
View reviewed changes
Copy link

@jcantrell jcantrell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Built, tested, run. That was fun. +1

Jafosterja
Jafosterja approved these changes Feb 27, 2019
View reviewed changes
Copy link

@Jafosterja Jafosterja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Built and tested. Looks good.

@tim-pugh tim-pugh merged commit be0971b into develop Feb 27, 2019
@connorkuehl connorkuehl deleted the skip-unions branch February 27, 2019 22:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jcantrell jcantrell jcantrell approved these changes

@Jafosterja Jafosterja Jafosterja approved these changes

@Nixoncole Nixoncole Awaiting requested review from Nixoncole

@tim-pugh tim-pugh Awaiting requested review from tim-pugh

@nforbus nforbus Awaiting requested review from nforbus

@jeffreytakahashi jeffreytakahashi Awaiting requested review from jeffreytakahashi

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@connorkuehl @jcantrell @Jafosterja @tim-pugh