Closed Nixoncole closed 5 years ago
Ah I'll be honest I did not know there was formalized acceptance criteria. I'll look around and see about emitting compiler warnings etc.
Let's add this PR to a develop branch (a different one then the current one named develop :-) ) so we can prep for the RFC, but besides that I'll have to take a look at this tomorrow/when the rest of the bits come on in.
Let's add this PR to a develop branch (a different one then the current one named develop :-) )
I might be mistaken, but I thought we were branching off only for features that would be merged AFTER we instate the code freeze. Is that correct?
Let's add this PR to a develop branch (a different one then the current one named develop :-) )
I might be mistaken, but I thought we were branching off only for features that would be merged AFTER we instate the code freeze. Is that correct?
We've talked about this last Tueday
"We'll call a code freeze, send rfc, have a develop branch for the stretch, fix the feedback, and can resubmit again"
If a stretch goals is all good then we'll just do a pr from that branch (or branches) into our mainline before the code freeze. That is fine.
Just added code that implements first attempt at emitting a compiler warning when a struct is tagged with both attributes "randomize_layout" and "no_randomize_layout".
Building now for test
Just to let everyone know what the state of this is, in case they want to help...
This currently builds, and our only implementation additions are supposedly barebones for emitting a compiler warning. The issue is that when trying to compile a file, I receive...
NixonCole@Coles-MacBook-Pro-2 ~/clangRandStruct $ llvm-project/build/bin/clang poc.c -o poc
poc.c:2:10: fatal error: 'stdio.h' file not found
#include <stdio.h>
^~~~~~~~~
1 error generated.
When I compile with my system's clang
(Mac OS), it compiles without issue. This leads me to believe that I have broken something somehow with my implementation.
poc.c:2:10: fatal error: 'stdio.h' file not found
This leads me to believe that I have broken something somehow with my implementation.
This doesn't strike me as a regression in your code. It looks like the compiler can't find your standard library. I don't know how to verify where or how or if it's properly installed on Mac. Try running your compile command with -v
(ex: path/to/clang -v poc.c
. You'll want to look for parts in the text where it says something like:
#include "..." search starts here:
#include <...> search starts here:
/usr/local/include
/home/kuehlcon/llvm-project/build/lib/clang/9.0.0/include
/usr/include/x86_64-linux-gnu
/usr/include
I checked out your branch and tried it out for myself.
Here is the version of poc.c
:
#include <stdio.h>
struct mystruct {
char *first;
char *second;
}__attribute__((no_randomize_layout)) __attribute__((randomize_layout));
int main(void)
{
struct mystruct m;
m.first = "I'm the first!";
m.second = "and I'm the second!";
printf("%s\n%s\n", *(&m), *(&m + sizeof(char *)));
return 0;
}
Here's the compile:
/home/kuehlcon/llvm-project/build/bin/clang -g -Wno-format poc.c -o rand
poc.c:3:8: warning: struct declared with 'randomize_layout' and 'no_randomize_layout'
attributes. attribute 'no_randomize_layout' takes precedence
struct mystruct {
^
1 warning generated.
Checking structure layout with pahole
shows no difference :+1:
As well as a quick run to make sure nothing's happened:
kuehlcon@randstruct-dev:~/randstruct-testing/poc$ ./rand
I'm the first!
and I'm the second!
kuehlcon@randstruct-dev:~/randstruct-testing/poc$ ./reg
I'm the first!
and I'm the second!
Nice work! I'll go through and update my review.
I have no idea why Github is displaying all these commits in this PR; they've already landed in develop
.
It's easier to review the PR by seeing the branch diff here.
This looks pretty good if you ask me! Thank you for providing the link to an easier view.
Closes #6
Please ignore my poorly named git branch...
Outside of that, I think this should work. Next is docs, which will touch both of these files again.
This doesnt have to be merged if you dont want to @connorkuehl but here ya go