On Tue, Aug 21, 2012 at 11:36 AM, Kostya Serebryany <[email protected]> wrote:
> submitted the clang part: r162259. > Looking at the rest (there are minor problems, I may be able to fix them > myself). > LLVM part went as r162268 with minor changes: 1. Don't call createInitializerPoisonCalls if there are no globals with dynamic init in the current TU. 2. Fix test instrument_initializer_metadata.ll to have correct metadata name 3. removed instrument_initializer.ll -- I did not understand why this test was needed. Please comment. Looking at the compiler-rt part, where I don't like some parts. --kcc > > > --kcc > > > On Tue, Aug 21, 2012 at 12:52 AM, Reid Watson <[email protected]> wrote: > >> OK, I re-reviewed what was contained in the patches. >> I had a few non-reviewed experiments accidentally included in the >> previous patches. >> I've attached the correct versions, containing only the properly >> reviewed changes. >> Sincere apologies for the trouble! >> >> On Mon, Aug 20, 2012 at 1:14 PM, Reid Watson <[email protected]> wrote: >> > Sorry, minor problem with the compiler-rt patch. >> > >> > Please commit the attached version of the compiler-rt patch instead of >> > the previous one. >> > >> > On Mon, Aug 20, 2012 at 10:57 AM, Reid Watson <[email protected]> wrote: >> >> I've incorporated all the review comments in the attached patches. >> >> The tool is off by default, and enabled by adding -mllvm >> >> -asan-initialization-order to clang. >> >> >> >> I don't have commit access, so I'd appreciate it if someone could >> >> commit these for me. >> >> >> >> All the best, >> >> Reid >> >> >> >> On Thu, Aug 16, 2012 at 6:09 PM, Reid Watson <[email protected]> wrote: >> >>> I'll add in the prefix before the final version for commit. >> >>> >> >>> The goal is to detect globals with dynamic initialization per the C++ >> >>> standard, so that ASan knows which globals can only be accessed during >> >>> . It's valid to access a statically initialized global inside of an >> >>> initializer, even if that global resides in a different TU. Thus, >> >>> LLVM/ASan need to be able to distinguish between >> >>> dynamically/statically initialized globals. I'm not certain this is >> >>> optimal, but there haven't been any unexpected false positives with >> >>> it. We may be missing a few case, and the optimizer can certainly end >> >>> up leaving us with false negatives, but those are much better than >> >>> false positives. >> >>> >> >>> On Thu, Aug 16, 2012 at 5:27 PM, Eric Christopher <[email protected]> >> wrote: >> >>>> >> >>>> >> >>>> On Aug 16, 2012, at 1:05 AM, Kostya Serebryany <[email protected]> >> wrote: >> >>>> >> >>>>> +llvm-commits >> >>>>> >> >>>>> Reid, >> >>>>> >> >>>>> The LLVM and compiler-rt patches look good. >> >>>>> Please fix the remaining small issues (see my code review comments) >> and commit. >> >>>>> Hold on with the output tests for a bit since Alexey Samsonov is >> migrating them to cmake (please coordinate with him and commit as a >> separate patch). >> >>>>> >> >>>>> The stress test should contain X files, Y linker initialized >> globals and Z dynamically initialized globals. >> >>>>> Such test only makes sense where all 3 numbers are large. >> >>>>> I guess you can commit a single .sh script into >> compiler-rt/lib/asan/scripts >> >>>> >> >>>> The metadata should at least be prefixed with something like >> llvm.asan.<whatever> instead of just the name. That way it's more >> identifiable. >> >>>> >> >>>> What's the idea behind the metadata use anyhow? >> >>>> >> >>>> -eric >> > >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
