[llvm-commits] LLVM changes for detecting initialization order problems in Address Sanitizer

[Kostya Serebryany]

Hi Reid,

+// This flag may need to be replaced with -fasan-initializers.
Ideally, this phase should be on-by default when asan is enabled.
Assuming it will be robust and have no false reports, of course.
For now, -mllvm -asan-initializers is ok for me.


   size_t n = GlobalsToChange.size();


+  NumGlobals = n;

Please remove 'n'.

+    if (ClInitializers && F->getName() == "_GLOBAL__I_a")



This is wrong. We should not identify global init functions using the name.
There should be some other way (attribute? metadata?)

+  // Add a call to poison all external globals before the given function
starts


Ouch. This will make the algorithm quadratic by the number of global
variables.
Essentially, we poison all globals each time we initialize a global.
Instead, we need to poison all globals of all modules (except for the
current) when we start handling a given module.
This will make it quadratic by the number of modules, which is tolerable.

Besides, I don't see how you distinguish between globals that have run-time
init and linker-initialized-globals.
In particular, the following is sad:
+  if (isa<GlobalVariable>(Addr) && ClOpt && ClOptGlobals &&
!ClInitializers) {

Here you disable a performance optimization that (last time I measured)
gave ~1% of speedup.
Meanwhile, we still don't need to touch linker-initialized globals.

May I ask you to send next asan patches using rietveld (in addition to
attaching the patch to the message)?
I know that LLVM community does not require that, but my code review
 productivity drops when I look at raw patches.

Thanks!





On Sat, Jun 30, 2012 at 1:36 AM, Reid Watson <reidw at google.com> wrote:

> Hello,
>
> I've attached a patch to add basic support for detecting problems with
> initialization order in C++ to AddressSanitizer.
> This patch relies on changes to the runtime library, and I've sent a
> patch for this to cfe-commits.
> This is definitely a first draft, and it leans toward false positives
> (function local statics, in particular), but it does detect the most
> basic cases of the "static initialization order fiasco".
> For now, I'd like to get a working baseline/infrastructure committed,
> in order to avoid any monster commits.
>
> All the best,
> Reid
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120702/d2f63ff7/attachment.html>