<div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Sun, Aug 21, 2016 at 7:18 AM Stefan Kempf <<a href="mailto:sisnkemp@gmail.com">sisnkemp@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Tim Shen wrote:<br>
> On Fri, Aug 19, 2016 at 10:28 AM Stefan Kempf <<a href="mailto:sisnkemp@gmail.com" target="_blank">sisnkemp@gmail.com</a>> wrote:<br>
><br>
> > If M.getOrInsertGlobal() does not return with a GlobalVariable,<br>
> > __guard_local was declared with a different type than long. This<br>
> > is a programming mistake. I decided against the assert here<br>
> > because the linker should catch this, and I wanted to avoid triggering<br>
> > asserts because of programming errors.<br>
> ><br>
><br>
> Can you elaborate on "programming mistake"?<br>
<br>
__guard_local is a reserved identifier and is supposed to be defined as<br>
long in the startup code or the kernel only. But clang and OpenBSD's gcc<br>
will happily compile a piece of source code that violates this, e.g. by<br>
declaring __guard_local as char, or float (=> "programming mistake").<br></blockquote><div><br></div><div>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.</div><div><br></div><div>If by calling Module::getOrInsertGlobal() inserts a `i8** __guard_local`, then it's llvm's fault by mistaking the return type.</div><div><br></div><div>I wonder if we can do:</div><div>if (IsOpenBSD) {</div><div>  // Insert the right declaration.</div><div>  <span style="line-height:1.5">M.getOrInsertGlobal("__guard_local", CLongType);</span></div><div><span style="line-height:1.5">  // A lazy way to say "cast".</span></div><div><span style="line-height:1.5">  return M.getOrInsertGlobal("__guard_local", i8Ptr);</span></div><div>} else {</div><div>  // do the normal thing</div><div>}</div><div><br></div><div>But I honestly don't know how to get "CLongType" in LLVM. Maybe Clang should generate such a declaration. <span style="line-height:1.5">Anyways, this patch doesn't make anything worse, so I'm happy to accept it.</span></div><div><div><span style="line-height:1.5"><br></span></div><div><span style="line-height:1.5">Also, can you point me the place GCC implements this? I searched for "__guard_local" but didn't find anything.</span><br></div></div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">For such a program, M.getOrInsertGlobal will return a cast operation<br>
instead of a GlobalValue.<br>
<br>
My understanding was that asserts are used to check for internal<br>
compiler errors. If something is wrong in the input source code,<br>
normal warnings or errors should be emitted.<br>
<br>
Currently the linker stage prevents that an incorrectly redefined<br>
__guard_local is used in the stack protector. Maybe clang could<br>
check this in the future. But I think for the moment we can get away<br>
without additional diagnostics here, and save this for a separate diff.<br>
</blockquote></div></div>