[LLVM] Uninitialized member in copy constructor in DataLayout.h

[David Blaikie]

On Wed, Feb 27, 2013 at 11:59 AM, Peng Cheng <gm4cheng at gmail.com> wrote:
> I do not have the commit access.  Please commit that.

Committed as r176213

> One question:  How can I get the commit access?  I am a heavy llvm user, and
> might send in more patches.

The process is documented here: http://llvm.org/docs/DeveloperPolicy.html

>
>
> On Wednesday, February 27, 2013, David Blaikie wrote:
>>
>> On Wed, Feb 27, 2013 at 11:33 AM, Peng Cheng <gm4cheng at gmail.com> wrote:
>> > I cannot create an llvm ir for regression test.  I test the llvm ir,
>> > which
>> > causes the uninitialized value in my program, with llc, but cannot
>> > reproduce
>> > the issue with valgrind.  I think that it is related to the order and
>> > types
>> > of different ir transformations between llc and my program, or llc goes
>> > to
>> > the branch to create DataLayout directly from Module, not from copy
>> > constructor. Maybe someone familiar with the ir transformation details
>> > could
>> > come up a test case.
>> >
>> > But if you follows the llvm tutorial at
>> > http://llvm.org/docs/tutorial/LangImpl4.html
>> >
>> > OurFPM.add(new DataLayout(*TheExecutionEngine->getDataLayout()));
>>
>> Might be nice to have the example code checked in somewhere - not sure
>> where it would go/how it would be tested.
>>
>> Anyway, I'll sign off on this - please commit the change. If you don't
>> have commit access I can do it for you.
>>
>> > This would put a bomb in your code.  When you were lucky,
>> > StackNaturalAlign
>> > is given some value and program works; otherwise, lead to undefined
>> > behaviors and days to debug...
>> >
>> >
>> > On Wed, Feb 27, 2013 at 12:05 AM, David Blaikie <dblaikie at gmail.com>
>> > wrote:
>> >>
>> >> On Tue, Feb 26, 2013 at 8:37 PM, Peng Cheng <gm4cheng at gmail.com> wrote:
>> >> > The usual llvm regression tests for a standard constructor bug seems
>> >> > like an
>> >> > overkill, leading to many wasteful code exercises, such as ir
>> >> > reading,
>> >> > optimization, code generation, ...
>> >>
>> >> While I'm the last to advocate for indirect testing, this is generally
>> >> the attitude in the LLVM project - to exercise from the actual
>> >> user-programs. This ensures better coverage (though not necessarily
>> >> actually /testing/ all of that, of course) in some ways, worse
>> >> coverage in others.
>> >>
>> >> > Are there any other places to place the unit tests for classes in
>> >> > llvm
>> >> > code
>> >> > base?  Simply to test whether all the necessary fields are
>> >> > initialized
>> >> > after
>> >> > the copy constructor.
>> >>
>> >> We do have true API-level unit tests as well - just not often written,
>> >> usually only for core/generic data structures & the like.
>> >>
>> >> >
>> >> >
>> >> > On Tue, Feb 26, 2013 at 10:51 PM, David Blaikie <dblaikie at gmail.com>
>> >> > wrote:
>> >> >>
>> >> >> On Tue, Feb 26, 2013 at 7:34 PM, Peng Cheng <gm4cheng at gmail.com>
>> >> >> wrote:
>> >> >> > It did not fail on any valgrind bots.  But when I used valgrind to
>> >> >> > check
>> >> >> > my
>> >> >> > jit engine based on llvm, I found the bug.
>> >> >> >
>> >> >> > Could you point to me an example of test case for the valgrind
>> >> >> > bot?
>> >> >> > I
>> >> >> > need
>> >> >> > to see how the test infrastructure is setup in llvm.
>> >> >>
>> >> >> The current public valgrind bot (
>> >> >> http://lab.llvm.org:8011/builders/clang-x86_64-linux-vg - there may
>> >> >> be
>> >> >> others) just runs the usual LLVM/Clang regression tests. So if you
>> >> >> can
>> >> >> construct a nice small/simple regression test you can add that (&
>> >> >> limit the test to only running under valgrind, potentially)
>> >> >>
>> >> >> > On Tue, Feb 26, 2013 at 10:28 PM, David Blaikie
>> >> >> > <dblaikie at gmail.com>
>> >> >> > wrote:
>> >> >> >>
>> >> >> >> On Tue, Feb 26, 2013 at 7:15 PM, Peng Cheng <gm4cheng at gmail.com>
>> >> >> >> wrote:
>> >> >> >> > The copy constructor of class DataLayout fails to initialize
>> >> >> >> > StackNaturalAlign.  It could lead to conditional jump on
>> >> >> >> > uninitialized
>> >> >> >> > variables in member function exceedsNaturalStackAlignment.
>> >> >> >>
>> >> >> >> Does this fail any valgrind bots? Could you write a test case
>> >> >> >> that
>> >> >> >> would fail under valgrind?
>> >> >> >>
>> >> >> >> > See fix in the patch attached.
>> >> >> >> >
>> >> >> >> > _______________________________________________
>> >> >> >> > llvm-commits mailing list
>> >> >> >> > llvm-commits at cs.uiuc.edu
>> >> >> >> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>> >> >> >> >
>> >> >> >
>> >> >> >
>> >> >
>> >> >
>> >
>> >