[valgrind] [Bug 356044] Dwarf line info reader misinterprets is_stmt register

[Ivo Raisr via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=356044

--- Comment #9 from Ivo Raisr  ---
Yes, '&&' seems misplaced. I attached another patch which fixes this style
issues.

As regards possible additional merges in canonicaliseLoctab() - preliminary
investigation shows that some gains are possible. However I would like to
tackle this separately, to avoid combining too many things in one commit.

-- 
You are receiving this mail because:
You are watching all bug changes.

[valgrind] [Bug 356044] Dwarf line info reader misinterprets is_stmt register

[Ivo Raisr via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=356044

Ivo Raisr  changed:

   What|Removed |Added

  Attachment #95828|0   |1
is obsolete||

--- Comment #8 from Ivo Raisr  ---
Created attachment 95895
  --> https://bugs.kde.org/attachment.cgi?id=95895=edit
patch with formatting fixed

Patch with fixed formatting.

-- 
You are receiving this mail because:
You are watching all bug changes.

[valgrind] [Bug 356044] Dwarf line info reader misinterprets is_stmt register

[Ivo Raisr via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=356044

Ivo Raisr  changed:

   What|Removed |Added

 Resolution|--- |FIXED
 Status|UNCONFIRMED |RESOLVED
  Latest Commit||r15741

--- Comment #10 from Ivo Raisr  ---
Fixed in SVN commit r15741.

-- 
You are receiving this mail because:
You are watching all bug changes.

[valgrind] [Bug 356044] Dwarf line info reader misinterprets is_stmt register

[Ivo Raisr via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=356044

--- Comment #11 from Ivo Raisr  ---
See also:
Bug 356273 - conserve memory by merging adjacent DiLoc entries in the debug
info location table

-- 
You are receiving this mail because:
You are watching all bug changes.

[valgrind] [Bug 356044] Dwarf line info reader misinterprets is_stmt register

[Philippe Waroquiers via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=356044

--- Comment #7 from Philippe Waroquiers  ---
(In reply to Ivo Raisr from comment #6)
> Created attachment 95828 [details]
> proposed patch
> 
> Adjacent DiLoc entries are now merged if they refer to the same line. This
> should give an improvement in terms of memory used.

Yes, results are good. With the patch, the memory  used is now similar to the
trunk.
Just one question: the merging is done when adding a new entry in the loctab
(i.e. in addLoc function). That is good to avoid uselessly growing the loctab 
during insertion.

I am wondering however if this merges all what can be merged.
Maybe it would be useful to also merge adjacent entries in canonicaliseLoctab
(after having sorted on addr) ?
This is of course only useful if there are non successive addLoc calls for
mergeable entries.

Otherwise, small style remark for the storage.c patch: I think (most of) the
code
splits the too long lines before the &&, not after.
so this
+  if ((previous->lineno == loc->lineno) &&
+  (previous->addr + previous->size == loc->addr)) {
should be
+  if ((previous->lineno == loc->lineno)
+  && (previous->addr + previous->size == loc->addr)) {

-- 
You are receiving this mail because:
You are watching all bug changes.

[valgrind] [Bug 356044] Dwarf line info reader misinterprets is_stmt register

[Ivo Raisr via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=356044

--- Comment #4 from Ivo Raisr  ---
So I spotted the problem by a chance, because another redirected function got
used on Solaris/x86+amd64 (using gcc 4.8.2). However this is a generic problem
with location info on Linux as well.

Consider the following decoded "Line Number Statements" as displayed by
"readelf --debug-dump=rawline":
  (previous instructions were related to line 868)
  ...
  Set is_stmt to 1
  Special opcode 174: advance Address by 12 to 0x1c72 and Line by 1 to 869
  Advance PC by constant 17 to 0x1c83
  Special opcode 5: advance Address by 0 to 0x1c83 and Line by 0 to 869
  Extended opcode 4: set Discriminator to 1
  Set is_stmt to 0
  Special opcode 131: advance Address by 9 to 0x1c8c and Line by 0 to 869
  Extended opcode 4: set Discriminator to 2
  Special opcode 75: advance Address by 5 to 0x1c91 and Line by 0 to 869
  Extended opcode 4: set Discriminator to 1
  Special opcode 75: advance Address by 5 to 0x1c96 and Line by 0 to 869
  Extended opcode 4: set Discriminator to 2
  Copy
  Extended opcode 4: set Discriminator to 1
  Special opcode 131: advance Address by 9 to 0x1c9f and Line by 0 to 869
  Extended opcode 4: set Discriminator to 2
  Advance PC by constant 17 to 0x1cb0
  Special opcode 89: advance Address by 6 to 0x1cb6 and Line by 0 to 869
  Extended opcode 4: set Discriminator to 1
  Special opcode 159: advance Address by 11 to 0x1cc1 and Line by 0 to 869
  Extended opcode 4: set Discriminator to 2
  Special opcode 131: advance Address by 9 to 0x1cca and Line by 0 to 869
  Special opcode 131: advance Address by 9 to 0x1cd3 and Line by 0 to 869
  Extended opcode 4: set Discriminator to 3
  Special opcode 117: advance Address by 8 to 0x1cdb and Line by 0 to 869
  Extended opcode 4: set Discriminator to 1
  Advance PC by 88 to 0x1d33
  Special opcode 5: advance Address by 0 to 0x1d33 and Line by 0 to 869
  Extended opcode 4: set Discriminator to 2
  Advance PC by constant 17 to 0x1d44
  Special opcode 47: advance Address by 3 to 0x1d47 and Line by 0 to 869
  Advance PC by 12 to 0x1d53
  Extended opcode 1: End of Sequence

Line 869 corresponds to function _vgr10110ZU_VgSoSynsomalloc_memalign() - which
happens to be the last one in this compilation unit. Its size is 0xe1 (225)
bytes as confirmed by nm, readelf and also disassembly.

However current Valgrind functionality adds DiLoc entries only for addresses
where "is_stmt = 1", that is address range
0x1c72-0x1c83 (17 bytes). This is clearly wrong; address ranges for "is_stmt =
0" should be covered as well.

-- 
You are receiving this mail because:
You are watching all bug changes.