XE7 and more With weirdness

-
XE7 and more With weirdness

100% reproducable fail: Access violation at address 00B5464D in module 'MyApp.exe'. Read of address 00000040.

Fails:
    tpLot := Tp.Add;
    with tpLot do
    begin
      LotId := Lot.Id; // <--BANG
      LotNo := Lot.LotNumber;
      ExpiryDate := Lot.ExpireDate;
      ProducedDate := Lot.ProducedStartTime;
      ArticleId := ArtSum.ArticleId;
      ArticleNo := ArtSum.ArticleNumber;
      OriginalArticleId := ArtSum.ArticleId;
      Quantity := NumDPacks * ArtSum.BaseUnitsPerKPack;
      NetWeight := Quantity * ArtSum.NetWeightPrBaseUnit;
      GrossWeight := NetWeight +
        (ArtSum.TaraWeightPrFPack * Quantity);
      LotLockState := Lot.LockStateCode;
    end;

Works:
    tpLot := Tp.Add;
    tpLot.LotId := Lot.Id;
    tpLot.LotNo := Lot.LotNumber;
    tpLot.ExpiryDate := Lot.ExpireDate;
    tpLot.ProducedDate := Lot.ProducedStartTime;
    tpLot.ArticleId := ArtSum.ArticleId;
    tpLot.ArticleNo := ArtSum.ArticleNumber;
    tpLot.OriginalArticleId := ArtSum.ArticleId;
    tpLot.Quantity := NumDPacks * ArtSum.BaseUnitsPerKPack;
    tpLot.NetWeight := tpLot.Quantity * ArtSum.NetWeightPrBaseUnit;
    tpLot.GrossWeight := tpLot.NetWeight + (ArtSum.TaraWeightPrFPack * tpLot.Quantity);
    tpLot.LotLockState := Lot.LockStateCode;


Lot and ArtSum are properly initated and readable (inspected in debugger), so I assume it is the assignment that fails.
The problem is, I cannot reproduce in a mini-example.

If I wasn't set at eradicating all with statements before, I surely am now.
#sigh

Comments

  1. Bill MeyerJanuary 7, 2015 at 8:00 AM

    I thought we all agreed "with" was a Bad Thing. ;)  I have, in other versions, seen errors thrown when you qualify a member inside the "with" with the item assigned in the "with". Messy writing about that...

    ReplyDelete
  2. Lars FosdalJanuary 7, 2015 at 8:03 AM

    We did agree, and it still is.  It just takes time to get rid of all that has accumulated over the years.  But a single with reference failing? That's bad!

    ReplyDelete
  3. Eric GrangeJanuary 7, 2015 at 8:13 AM

    Lars Fosdal Be wary you might just be getting rid of the canary, or shooting the messenger, or breaking the thermometer ;)

    Everything else being correct, this failure could just be a sign of other memory corruption/stack overwrites, and removing the "with" hides that issue under the carpet.
    The fact you cannot reproduce it in a simpler case is another hint in that direction.

    Inspect in the asm/CPU view, and debug the state there: if there is a memory corruption of some sort, the higher level debugger is unreliable.

    Then, once that's solved, get rid of the "with".

    ReplyDelete
  4. Bill MeyerJanuary 7, 2015 at 8:14 AM

    Lars Fosdal Welcome to my world... I am removing them nearly every day. Big legacy project.

    ReplyDelete
  5. Walter PrinsJanuary 7, 2015 at 8:26 AM

    Might be worth a shot to try recompiling with FastMM with all checks enabled (double free etc etc) and see if that throws up anything.

    ReplyDelete
  6. Lars FosdalJanuary 7, 2015 at 8:37 AM

    Walter Prins - Did that, and it's solid.
    It was the assignment side that was broken.

    One thing I just noted - tpLot.LotNo had duplicates in the procedure parameters, but the with parameter should "out-scope" those.

    procedure SomeClass.DoThis(const ArticleNo, LotNo: String);
    var
      Lot: TLot;
      ArtSum: TArticleSummary;
      tp: TPack;
      tpLot: TPackLot;
    begin
      ArtSum := ArtSummaries.Locate(ArticleNo);
      Lot := Lots.Locate(LotNo);
      tp := TPack.Create;
      tpLot := tp.Add;
      with tpLot
      do begin
        LotId := Lot.Id;
        LotNo := Lot.LotNumber;
        ExpiryDate := Lot.ExpireDate;
        ProducedDate := Lot.ProducedStartTime;
        ArticleId := ArtSum.ArticleId;
        ArticleNo := ArtSum.ArticleNumber;
        OriginalArticleId := ArtSum.ArticleId;
        ...

    ReplyDelete
  7. Asbjørn HeidJanuary 7, 2015 at 8:39 AM

    If you assign Lot to a local variable named something else and use that inside with, does the with still bomb?

    ReplyDelete
  8. Lars FosdalJanuary 7, 2015 at 8:48 AM

    Given that Lot already is a local var, and not in duplicate with anything else, and found and returned by the locator - that seems far fetched.

    I'd give it a spin, but I've consumed all the testable data I received. I'll check  with the warehouse peeps if they can send me more pallet labels.

    ReplyDelete
  9. Asbjørn HeidJanuary 7, 2015 at 8:50 AM

    Lars Fosdal Well was just thinking if the With pulled in a different Lot.

    ReplyDelete
  10. David HeffernanJanuary 7, 2015 at 9:19 AM

    What you state doesn't tally with my understanding of scope. The variables brought into scope by that with are searched first. So the code emitted should be identical. The fact that you cannot make a repro suggests that the issue is on your end.

    ReplyDelete
  11. Lars FosdalJanuary 7, 2015 at 9:32 AM

    David Heffernan - When I put a breakpoint on the first assignment, I can inspect all properties of Lot, but when I do F8 to step to next line - I get the exception.  If the right side of the assignment was the issue, why does the problem go away when I take out the left side With?

    Unless the code generated is for assigning to a const LotNo string, and that somehow fails - which also would be a compiler error.

    ReplyDelete
  12. Daniela OsterhagenJanuary 7, 2015 at 9:51 AM

    Have you had a look at the generated asm code yet? It usually helps me tracking down what the actual problem is.

    ReplyDelete
  13. David HeffernanJanuary 7, 2015 at 10:35 AM

    To make progress, cut it down. Do you need all of the assignments? Or is one enough? Make a repro.

    ReplyDelete
  14. Joe C. HechtJanuary 8, 2015 at 7:00 PM

    Bill Meyer I agree. With is evil. The small optimizations perhaps generated by the compiler are not worth the frustrations. Having manned Borland's help desk. I could testify...

    ReplyDelete

Post a Comment