[PATCH] D11221: Fix crash when using __attribute__((section (".bss.name"))) incorrectly

Aaron Ballman aaron.ballman at gmail.com
Thu Jul 16 07:23:58 PDT 2015


On Thu, Jul 16, 2015 at 9:38 AM, Jakub Kuderski <jakub.kuderski at arm.com> wrote:
> kuhar added a subscriber: aaron.ballman.
> kuhar updated this revision to Diff 29896.
> kuhar added a comment.
>
> Thanks for review.
> Aaron - fixed things you pointed out, except not dropping attribute - I'm not entirely convinced, because attributes are being dropped on error in the entire file...

It's the correct approach. You only dropAttr when the attribute you
wish to drop is not the one you are currently adding. For instance,
from within a mergeFooAttr function when you want to drop an existing
attribute. The code as you have it written does all the work to add an
attribute that it then drops when it could instead determine that the
attribute is invalid before ever adding it.

~Aaron

>
> Richard:
> ad. 1. This should be fixed now - I only report error when variables are initialized with some 'constant expression', so that dynamic initializers are allowed.
> ad. 2.
>
>> I think it's less reasonable to do this in C++>
>
>
> Could you explain why?
> ad. 3. I didn't know what to do with member pointers, so I left them unchecked. Is there a better way to determine non-zeroness of variable? Or maybe I should add it somewhere, so it can be target-specific (CXXABI.h?)?
> ad. 4. Are there some existing mechanisms to do so, or could explain how it should be done? I've only found a function for validating section names (`isValidSectionSpecifier` in TargetInfo.h).
> I would really appreciate your further help on this.
>
>
> Repository:
>   rL LLVM
>
> http://reviews.llvm.org/D11221
>
> Files:
>   include/clang/Basic/DiagnosticSemaKinds.td
>   lib/Sema/SemaDecl.cpp
>   lib/Sema/SemaDeclAttr.cpp
>   test/Sema/attr-section-bss.c
>



More information about the cfe-commits mailing list