[PATCH] D23674: Do not set __guard_local to hidden for OpenBSD SSP

[Tim Shen via llvm-commits]

On Sun, Aug 21, 2016 at 7:18 AM Stefan Kempf <sisnkemp at gmail.com> wrote:

> Tim Shen wrote:
> > On Fri, Aug 19, 2016 at 10:28 AM Stefan Kempf <sisnkemp at gmail.com>
> wrote:
> >
> > > If M.getOrInsertGlobal() does not return with a GlobalVariable,
> > > __guard_local was declared with a different type than long. This
> > > is a programming mistake. I decided against the assert here
> > > because the linker should catch this, and I wanted to avoid triggering
> > > asserts because of programming errors.
> > >
> >
> > Can you elaborate on "programming mistake"?
>
> __guard_local is a reserved identifier and is supposed to be defined as
> long in the startup code or the kernel only. But clang and OpenBSD's gcc
> will happily compile a piece of source code that violates this, e.g. by
> declaring __guard_local as char, or float (=> "programming mistake").
>

A normal piece of source code doesn't declare __guard_local, does it? The
user only smash a button called "-fstack-protector" on a normal piece of
code, and the compiler should do the right thing, including generating
`long __guard_local` declaration for the current compilation unit, then
generate SSP code, etc.

If by calling Module::getOrInsertGlobal() inserts a `i8** __guard_local`,
then it's llvm's fault by mistaking the return type.

I wonder if we can do:
if (IsOpenBSD) {
  // Insert the right declaration.
  M.getOrInsertGlobal("__guard_local", CLongType);
  // A lazy way to say "cast".
  return M.getOrInsertGlobal("__guard_local", i8Ptr);
} else {
  // do the normal thing
}

But I honestly don't know how to get "CLongType" in LLVM. Maybe Clang
should generate such a declaration. Anyways, this patch doesn't make
anything worse, so I'm happy to accept it.

Also, can you point me the place GCC implements this? I searched for
"__guard_local" but didn't find anything.

For such a program, M.getOrInsertGlobal will return a cast operation
> instead of a GlobalValue.
>
> My understanding was that asserts are used to check for internal
> compiler errors. If something is wrong in the input source code,
> normal warnings or errors should be emitted.
>
> Currently the linker stage prevents that an incorrectly redefined
> __guard_local is used in the stack protector. Maybe clang could
> check this in the future. But I think for the moment we can get away
> without additional diagnostics here, and save this for a separate diff.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160821/6f15a45f/attachment.html>