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

[Peng Cheng]

I do not have the commit access.  Please commit that.

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

On Wednesday, February 27, 2013, David Blaikie wrote:

> On Wed, Feb 27, 2013 at 11:33 AM, Peng Cheng <gm4cheng at gmail.com<javascript:;>>
> 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<javascript:;>>
> wrote:
> >>
> >> On Tue, Feb 26, 2013 at 8:37 PM, Peng Cheng <gm4cheng at gmail.com<javascript:;>>
> 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<javascript:;>
> >
> >> > wrote:
> >> >>
> >> >> On Tue, Feb 26, 2013 at 7:34 PM, Peng Cheng <gm4cheng at gmail.com<javascript:;>>
> 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 <javascript:;>>
> >> >> > wrote:
> >> >> >>
> >> >> >> On Tue, Feb 26, 2013 at 7:15 PM, Peng Cheng <gm4cheng at gmail.com<javascript:;>
> >
> >> >> >> 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 <javascript:;>
> >> >> >> > 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/20130227/2d6bfb3d/attachment.html>