<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Sat, Jan 30, 2016 at 9:00 AM, Vassil Vassilev <span dir="ltr"><<a href="mailto:v.g.vassilev@gmail.com" target="_blank">v.g.vassilev@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
  
    
  
  <div bgcolor="#FFFFFF" text="#000000">
    <div>AFAICT the making a test case
      independent on STL is the hard part. I think it will be always
      failing due to the relocation of the memory region of the
      underlying SmallVector.</div></div></blockquote><div><br></div><div>Sorry, I think I didn't explain myself well. What I mean is - due to the instability of test cases for UB (especially library UB), we don't usually commit test cases for them - we just fix them without tests. About the only time I think committing a test is helpful for UB (especially library UB) is if we have a reliable way to test/catch the UB (eg: the sanitizers or a checking STL that fails fast on validation violations). So, while it would be good to provide/demonstrate your test case, I would not commit the test case unless it's a minimal reproduction in the presence of one of those tools (sanitizers or checking STL) - and would just commit the fix without a test case, usually. <br><br>(I'm not actually reviewing this patch - I don't know much about the modules code, but just providing a bit of context and first-pass-review)</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div bgcolor="#FFFFFF" text="#000000"><div><div><div class="h5"><br>
      On 30/01/16 17:37, David Blaikie wrote:<br>
    </div></div></div><div><div class="h5">
    <blockquote type="cite">
      <p dir="ltr">Yeah, it's good to have a reproduction, but I
        wouldn't actually commit one - unless you have a checked stl to
        test against that always fails on possibly-invalidating
        sequences of operations, then you'd have a fairly simple
        reproduction</p>
      <div class="gmail_quote">On Jan 30, 2016 8:23 AM, "Vassil
        Vassilev" <<a href="mailto:v.g.vassilev@gmail.com" target="_blank">v.g.vassilev@gmail.com</a>>
        wrote:<br type="attribution">
        <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
          <div bgcolor="#FFFFFF" text="#000000">
            <div>Sorry.<br>
              <br>
              Our module builds choke on merging several modules,
              containing declarations from STL (we are using libc++, no
              modulemaps).<br>
              <br>
              When writing a new module containing the definition of a
              STL reverse_iterator, it collects all its template
              specializations. Some of the specializations need to be
              deserialized from a third module. This triggers an
              iterator invalidation bug because of the deserialization
              happening when iterating the specializations container
              itself. This happens only when the container's capacity is
              exceeded and the relocation invalidates the iterators and
              at best causes a crash (see valgrind reports in the bug
              report). Unfortunately I haven't been able to make the
              reproducer independent on STL.<br>
              <br>
              --Vassil<br>
              <br>
              On 30/01/16 17:08, David Blaikie wrote:<br>
            </div>
            <blockquote type="cite">
              <p dir="ltr">It might be handy to give an overview of the
                issue in the review (& certainly in the commit)
                message rather than only citing the bug number</p>
              <div class="gmail_quote">On Jan 30, 2016 6:49 AM, "Vassil
                Vassilev via cfe-commits" <<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>>
                wrote:<br type="attribution">
                <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Attaching
                  a fix to <a href="https://llvm.org/bugs/show_bug.cgi?id=26237" rel="noreferrer" target="_blank">https://llvm.org/bugs/show_bug.cgi?id=26237</a><br>
                  <br>
                  Please review.<br>
                  <br>
                  Many thanks!<br>
                  --Vassil<br>
                  <br>
                  _______________________________________________<br>
                  cfe-commits mailing list<br>
                  <a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
                  <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
                  <br>
                </blockquote>
              </div>
            </blockquote>
            <br>
          </div>
        </blockquote>
      </div>
    </blockquote>
    <br>
  </div></div></div>

</blockquote></div><br></div></div>