Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(757)

Issue 5416061: [asan] when -faddress-sanitizer is present, add required flags to the linker command (linux-only)

Can't Edit
Can't Publish+Mail
Start Review
Created:
13 years, 1 month ago by kcc
Modified:
13 years ago
CC:
daniel.dunbar, daniel_zuster.org
Base URL:
https://llvm.org/svn/llvm-project/cfe/trunk/
Visibility:
Public.

Patch Set 1 #

Patch Set 2 : comments #

Patch Set 3 : comments #

Total comments: 17

Patch Set 4 : simplify the patch #

Patch Set 5 : simplify the patch #

Patch Set 6 : just one file is here #

Total comments: 1

Patch Set 7 : comments #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+24 lines, -0 lines) Patch
M lib/Driver/Tools.cpp View 1 2 3 4 5 6 2 chunks +24 lines, -0 lines 1 comment Download

Messages

Total messages: 8
chandlerc
http://codereview.appspot.com/5416061/diff/3001/lib/Driver/Tools.cpp File lib/Driver/Tools.cpp (right): http://codereview.appspot.com/5416061/diff/3001/lib/Driver/Tools.cpp#newcode1094 lib/Driver/Tools.cpp:1094: static bool AsanIsEnabled(const ArgList &Args) { I don't really ...
13 years, 1 month ago (2011-11-22 00:36:08 UTC) #1
Evgeniy Stepanov
http://codereview.appspot.com/5416061/diff/3001/lib/Driver/Tools.cpp File lib/Driver/Tools.cpp (right): http://codereview.appspot.com/5416061/diff/3001/lib/Driver/Tools.cpp#newcode2500 lib/Driver/Tools.cpp:2500: addAsanLinkerFlagsForLinux(getToolChain(), Args, CmdArgs); On 2011/11/22 00:36:09, chandlerc wrote: > ...
13 years ago (2011-11-22 08:36:59 UTC) #2
kcc
http://codereview.appspot.com/5416061/diff/3001/lib/Driver/Tools.cpp File lib/Driver/Tools.cpp (right): http://codereview.appspot.com/5416061/diff/3001/lib/Driver/Tools.cpp#newcode1094 lib/Driver/Tools.cpp:1094: static bool AsanIsEnabled(const ArgList &Args) { On 2011/11/22 00:36:09, ...
13 years ago (2011-11-22 19:10:41 UTC) #3
chandlerc
Few more comments... http://codereview.appspot.com/5416061/diff/3001/lib/Driver/Tools.cpp File lib/Driver/Tools.cpp (right): http://codereview.appspot.com/5416061/diff/3001/lib/Driver/Tools.cpp#newcode1101 lib/Driver/Tools.cpp:1101: std::string res = TC.getDriver().Dir + "/../lib/clang/linux/"; ...
13 years ago (2011-11-22 23:14:35 UTC) #4
kcc
http://codereview.appspot.com/5416061/diff/3001/lib/Driver/Tools.cpp File lib/Driver/Tools.cpp (right): http://codereview.appspot.com/5416061/diff/3001/lib/Driver/Tools.cpp#newcode1101 lib/Driver/Tools.cpp:1101: std::string res = TC.getDriver().Dir + "/../lib/clang/linux/"; On 2011/11/22 23:14:35, ...
13 years ago (2011-11-22 23:26:52 UTC) #5
Evgeniy Stepanov
http://codereview.appspot.com/5416061/diff/3001/lib/Driver/Tools.cpp File lib/Driver/Tools.cpp (right): http://codereview.appspot.com/5416061/diff/3001/lib/Driver/Tools.cpp#newcode1101 lib/Driver/Tools.cpp:1101: std::string res = TC.getDriver().Dir + "/../lib/clang/linux/"; On 2011/11/22 23:14:35, ...
13 years ago (2011-11-24 11:48:53 UTC) #6
chandlerc
Only one real question... feel free to submit whenever, this can be addressed post-commit. http://codereview.appspot.com/5416061/diff/8003/lib/Driver/Tools.cpp ...
13 years ago (2011-11-30 00:26:16 UTC) #7
kcc
13 years ago (2011-11-30 00:37:25 UTC) #8
On 2011/11/30 00:26:16, chandlerc wrote:
> Only one real question... feel free to submit whenever, this can be addressed
> post-commit.
> 
> http://codereview.appspot.com/5416061/diff/8003/lib/Driver/Tools.cpp
> File lib/Driver/Tools.cpp (right):
> 
>
http://codereview.appspot.com/5416061/diff/8003/lib/Driver/Tools.cpp#newcode1111
> lib/Driver/Tools.cpp:1111: TC.AddCXXStdlibLibArgs(Args, CmdArgs);
> I don't understand this bit... Why do we need ta add the CXX standard library
> here? 

asan rt uses some of the C++ run-time. 
basically, a bit of STL. 
I think we can get rid of it with some effort, but now we do need it. 
 
> Is this safe to do when building C code?

I have not seen problems with C programs, but the only large C programs I've
built with asan were those from SPEC cpu2006. (someone else had success with
ffmpeg and perl)
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b