AaronBallman wrote:

> > If I'm understanding correctly, if you specify this on a local variable, it 
> > doesn't directly impact how the stack protector is emitted, just the 
> > heuristic for whether we choose to enable the stack protector? i.e. it has 
> > no effect with -fstack-protector-all?
> 
> That's right yeah. For the `-fstack-protector-all` case for example i believe 
> that is covered by the code in LLVM which looks for 
> `F->hasFnAttribute(Attribute::StackProtectReq)`, which is done prior to any 
> analysis on individual instructions. This PR and the equivalent LLVM one 
> change that instruction based analysis.
> 
> > Maybe we should use a different name for this? Using an existing attribute 
> > name is convenient, but the meaning here seems different enough that we 
> > should use something different. (This also makes it easier for users can 
> > easily find relevant documentation.) Maybe stack_protector_ignore or 
> > something like that.
> 
> Yeah I have no problem with a new name. Also considered something like 
> `no_implicit_stack_protector` perhaps, but maybe `stack_protector_ignore` is 
> better as otherwise folks have to think about what from my suggested name the 
> "implicit" is even tied to.
> 
> Happy to introduce `stack_protector_ignore` and update this if you think 
> that's the way to go.

Ope, I missed this comment thread when I was leaving review comments. Picking a 
new name for the attribute would make sense to me in this case given the 
landscape.

https://github.com/llvm/llvm-project/pull/173311
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to