[prev in list] [next in list] [prev in thread] [next in thread]
List: cfe-dev
Subject: Re: [cfe-dev] Stack Protectors
From: Bill Wendling <wendling () apple ! com>
Date: 2009-06-28 7:36:44
Message-ID: 4CBF7037-4969-42D4-B177-7B4F89527D16 () apple ! com
[Download RAW message or body]
Hi Eli,
I made those changes. Thanks for the review! Checked in as r74405.
-bw
On Jun 27, 2009, at 6:44 PM, Eli Friedman wrote:
> On Sat, Jun 27, 2009 at 6:16 PM, Bill Wendling<wendling@apple.com>
> wrote:
>> Hi,
>>
>> Here's a patch to implement stack protector support for Clang. This
>> doesn't
>> include test cases, which I will create before checking it. Could
>> someone
>> please review this?
>
> Review below:
>
>> Index: include/clang/Basic/TargetInfo.h
>> ===================================================================
>> --- include/clang/Basic/TargetInfo.h (revision 74401)
>> +++ include/clang/Basic/TargetInfo.h (working copy)
>> @@ -52,6 +52,7 @@
>> const char *UserLabelPrefix;
>> const llvm::fltSemantics *FloatFormat, *DoubleFormat,
>> *LongDoubleFormat;
>> unsigned char RegParmMax, SSERegParmMax;
>> + unsigned char StackProtector;
>>
>> // TargetInfo Constructor. Default initializes all fields.
>> TargetInfo(const std::string &T);
>
> This variable is unused; please get rid of it and the related code.
>
>> Index: lib/Driver/Tools.cpp
>> ===================================================================
>> --- lib/Driver/Tools.cpp (revision 74401)
>> +++ lib/Driver/Tools.cpp (working copy)
>> @@ -498,6 +498,14 @@
>> Args.AddLastArg(CmdArgs, options::OPT_fvisibility_EQ);
>> Args.AddLastArg(CmdArgs, options::OPT_fwritable_strings);
>>
>> + // Forward stack protector flags.
>> + if (Args.hasArg(options::OPT_fno_stack_protector))
>> + CmdArgs.push_back("--stack-protector=0");
>> + else if (Args.hasArg(options::OPT_fstack_protector))
>> + CmdArgs.push_back("--stack-protector=1");
>> + else if (Args.hasArg(options::OPT_fstack_protector_all))
>> + CmdArgs.push_back("--stack-protector=2");
>> +
>> // Forward -f options with positive and negative forms; we
>> translate
>> // these by hand.
>
> This doesn't match gcc's behavior; please add a new form of
> ArgList::getLastArg which takes three options as arguments and use it
> here.
>
> Besides those issues, the patch looks fine.
>
> -Eli
_______________________________________________
cfe-dev mailing list
cfe-dev@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic