Bug 252600 - [PATCH] Allow lhs to be a pointer for shl/shr
Summary: [PATCH] Allow lhs to be a pointer for shl/shr
Status: RESOLVED UNMAINTAINED
Alias: None
Product: valgrind
Classification: Developer tools
Component: sgcheck (show other bugs)
Version: 3.6 SVN
Platform: Compiled Sources Linux
: NOR normal
Target Milestone: ---
Assignee: Julian Seward
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-09-28 03:02 UTC by Eugene Toder
Modified: 2020-10-28 10:59 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
patch (5.56 KB, patch)
2010-09-28 03:08 UTC, Eugene Toder
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Eugene Toder 2010-09-28 03:02:53 UTC
At least some versions of gcc 4.x (I tried 4.1 and 4.4, so can be all of them) generate code with shr/shl when compiling alloca() without optimizations on x64, e.g.:

  4004e4:       55                      push   %rbp
  4004e5:       48 89 e5                mov    %rsp,%rbp
  4004e8:       53                      push   %rbx
  4004e9:       48 83 ec 28             sub    $0x28,%rsp
  4004ed:       89 7d dc                mov    %edi,-0x24(%rbp)
  4004f0:       48 83 ec 40             sub    $0x40,%rsp
  4004f4:       48 89 e0                mov    %rsp,%rax
  4004f7:       48 83 c0 0f             add    $0xf,%rax
  4004fb:       48 c1 e8 04             shr    $0x4,%rax
  4004ff:       48 c1 e0 04             shl    $0x4,%rax

Shr/shl is used to alight stack pointer to 16 bytes. At the moment ptrcheck does not allow pointer operands in shl & shr (the result is always treated as non-pointer) so access to alloca'd memory causes a error.

This patch adds support for shr/shl pointers -- lhs is allowed to be a pointer, shifting a pointer by a non-pointer results in a pointer; other cases result in a non-pointer as before.

I had to change more code than I wanted -- turns out rhs of shift operations can be of a different format than lhs. I handle this by assuming that non-word sized values can never be pointers. While at it I also made an  optimization to existing code -- when dealing with a constant value we can call nonptr_or_unknown() at instrumentation time instead of generating code that will repeatedly call it at run time with the same constant value.
Comment 1 Eugene Toder 2010-09-28 03:08:26 UTC
Created attachment 52036 [details]
patch
Comment 2 Julian Seward 2010-10-07 18:36:28 UTC
How carefully/extensively have you tested the patch?
On which platforms?
Comment 3 Eugene Toder 2010-10-07 18:50:00 UTC
I only tested on Linux x64. I ran ptrcheck tests, they don't seem very extensive and 2 of them fail for me on SVN head (stack traces are slightly different from expected). My patch does not change the results.
I also ran a large-ish program on which I found the issue -- it runs clean with the patch.