Skip to content
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

native: add address sanitizer make target #3327

Merged
merged 1 commit into from
Jul 9, 2015
Merged

native: add address sanitizer make target #3327

merged 1 commit into from
Jul 9, 2015

Conversation

miri64
Copy link
Member

@miri64 miri64 commented Jul 7, 2015

Adds a make target to compile an application with the GCC address sanitizer.

@miri64 miri64 added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Platform: native Platform: This PR/issue effects the native platform Area: tests Area: tests and testing framework labels Jul 7, 2015
@miri64 miri64 added this to the Release 2015.06 milestone Jul 7, 2015
@miri64
Copy link
Member Author

miri64 commented Jul 7, 2015

Might be also of interest for @LudwigOrtmann and @BytesGalore to look at.

OlegHahm
OlegHahm reviewed Jul 7, 2015
View reviewed changes
cpu/native/syscalls.c
@@ -162,7 +166,7 @@ void *calloc(size_t nmemb, size_t size)
}
else {
_native_in_calloc = 1;
*(void **)(&real_calloc) = dlsym(RTLD_NEXT, "calloc");
/* *(void **)(&real_calloc) = dlsym(RTLD_NEXT, "calloc"); */
Copy link
Member

Choose a reason for hiding this comment

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

Please explain why this is commented out and why it isn't removed?

Copy link
Member Author

Choose a reason for hiding this comment

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

You were commenting to fast ;-)

@miri64 miri64 force-pushed the native/enh/address-sanitizer branch from 17c64b5 to 6ee2534 Compare July 7, 2015 12:44
@miri64
Copy link
Member Author

miri64 commented Jul 7, 2015

Somehow the debugging symbols are missing for me… can somebody confirm? :/

@miri64 miri64 added the CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable label Jul 7, 2015
@miri64 miri64 force-pushed the native/enh/address-sanitizer branch from c459363 to 97d4ce5 Compare July 7, 2015 12:56
@miri64
Copy link
Member Author

miri64 commented Jul 7, 2015

Should I also add a target for the thread sanitizer?

@miri64
Copy link
Member Author

miri64 commented Jul 7, 2015

Somehow the debugging symbols are missing for me… can somebody confirm? :/

One needs a symbolizer (like llvm-symbolizer) for that. Sadly the ubuntu llvm-symbolizer already is differently called than in the referred link (llvm-symbolizer-3.4), so I need to think about how I will implement this. Any ideas are welcome

@miri64
Copy link
Member Author

miri64 commented Jul 7, 2015

Ahh there is a fix in GCC 4.9 for that.

@miri64
Copy link
Member Author

miri64 commented Jul 7, 2015

I'll document it in the wiki, as soon as this get's merged.

@PeterKietzmann
Copy link
Member

IIRC there is a problem with the thread system for 32 bit compiling. Will look at that again.

@miri64
Copy link
Member Author

miri64 commented Jul 7, 2015

You need to install libtsan:i386 probably...

PeterKietzmann
PeterKietzmann reviewed Jul 7, 2015
View reviewed changes
boards/native/Makefile.include
all-asan: export CFLAGS += -fsanitize=address -fno-omit-frame-pointer -g
all-asan: export CFLAGS += -DNATIVE_IN_CALLOC
all-asan: export LINKFLAGS += -fsanitize=address -fno-omit-frame-pointer -g
all-asan: export CFLAGS += -fsanitize=thread -fno-omit-frame-pointer -g
Copy link
Member

Choose a reason for hiding this comment

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

I think you mean all- t san

@miri64
Copy link
Member Author

miri64 commented Jul 7, 2015

You're right: there is no i386 support for libtsan in Debian and co. at least.

@PeterKietzmann
Copy link
Member

Ahja, that was it :-) ! Please squash. Would be nice to hear @LudwigOrtmann commenting this. Otherwise I tend to merge this soon.

@miri64 miri64 mentioned this pull request Jul 7, 2015
Merged
@PeterKietzmann
Copy link
Member

@authmillenon please squash

@miri64 miri64 force-pushed the native/enh/address-sanitizer branch from 0019301 to 8a33a62 Compare July 9, 2015 13:00
@miri64
Copy link
Member Author

miri64 commented Jul 9, 2015

Done.

@OlegHahm OlegHahm removed the CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable label Jul 9, 2015
@PeterKietzmann PeterKietzmann added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jul 9, 2015
@PeterKietzmann
Copy link
Member

@LudwigOrtmann I'll merge this now as it worked for me and Travis. Still it would be nice if you have a quick look at this

PeterKietzmann added a commit that referenced this pull request Jul 9, 2015
@PeterKietzmann
Merge pull request #3327 from authmillenon/native/enh/address-sanitizer
f95aaa6 
native: add address sanitizer make target
@PeterKietzmann PeterKietzmann merged commit f95aaa6 into RIOT-OS:master Jul 9, 2015
@miri64 miri64 deleted the native/enh/address-sanitizer branch July 9, 2015 14:07
@LudwigKnuepfer
Copy link
Member

@PeterKietzmann what exactly would you like my comment on?

@PeterKietzmann
Copy link
Member

I just wanted you to look over this PR, what you obviously did. So I'm fine

@miri64 miri64 mentioned this pull request Jul 13, 2015
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@LudwigKnuepfer LudwigKnuepfer

Labels
Area: tests Area: tests and testing framework CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: native Platform: This PR/issue effects the native platform Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
Projects
None yet
Milestone
Release 2015.09
Development

Successfully merging this pull request may close these issues.
4 participants
@miri64 @PeterKietzmann @LudwigKnuepfer @OlegHahm