Re: sdt-types tests

[Josh Stone <jistone at redhat dot com>]

On 11/22/2010 07:01 PM, Roland McGrath wrote:
> In commit ac28289, I added a usage of 'struct opaque *' as the type of a
> probe argument in sdt_types.c to test the PR12139 case.  But this change
> did not produce any regressions, despite such a probe being uncompilable
> with the current sys/sdt.h macros.  I can't figure out what that test is
> actually testing if it doesn't start failing when I introduce a probe use
> that won't compile.  So I do not understand the methodology of this test,
> unless I just did something stupid I am failing to notice.

I think Stan cleaned up some of the logic -- were you using the latest
version of the test at the time?  When I test on ac28289, I see V1/V2
are ok, and then failures in V3 like:

  FAIL: sdt_misc compiling types V3_uprobe

> In the later commit d079d75, I did the revamp of the sdt.h macros to handle
> the opaque pointer case (PR12139).  I'm still waiting for some C++ experts
> to get back to me about how kosher the C++ code there is, but it seems to
> work.
> 
> That produced no regressions, but since the problematic case didn't fail
> before, I can't tell whether that means anything at all either for fixing
> the new case or for actually not regressing on the old cases.  So if you
> are fixing the test methodology, then you should be sure to do that on a
> fork from before d079d75 so you can when you fix things so the PR12139
> failure actually fails, before testing the new sdt.h code.

As I said above, I just tried this manually and pre-fix it seems to fail
correctly.  And sorry to say, with your new sdt.h, it still fails.  If
you want to bypass whatever oddities our testsuite may have, just try
the thing directly:

> $ gcc testsuite/systemtap.base/sdt_types.c -c
> testsuite/systemtap.base/sdt_types.c: In function ‘main’:
> testsuite/systemtap.base/sdt_types.c:185:3: error: invalid use of undefined type ‘struct opaque’
> testsuite/systemtap.base/sdt_types.c:185:3: error: invalid use of undefined type ‘struct opaque’

That's an improvement - the old sdt.h gave four such errors. ;)  But
even the baby example given in PR12139 still fails in this way.  This is
on F14 x86_64, using gcc 4.5.1 20100924 (Red Hat 4.5.1-4).

> As I mentioned before, the new macros can produce sizes 1 or 2 as well as 4
> or 8.  For larger floating types and the large vector integer types (the
> ones called *128* or whatnot), it should produce larger numbers too, though
> I didn't try to test that.  So make sure the translator et al are not too
> surprised.
> 
> There's another new wrinkle for the translator/runtime that I forgot to
> mention.  On x86, it's now possible for the operands with sizes 1 or 2 to
> be the register name variants for those sizes, which never came up before.
> The 16-bit register names are just %ax and the like, omitting the 'r' or
> 'e' prefix.  The 8-bit register names are %[abcd][hl], with an 'l' and an
> 'h' variant for those four registers, l meaning the low 8 bits and h
> meaning the next 8 bits.  So the runtime needs to mask the extracted
> register value with 0xffff or 0xff, respectively, and for the h variants it
> needs to >>8 and then &0xff.  On x86-64, the numbered registers %rNN (the
> ones that don't exist at all on i386) also have 32-bit, 16-bit, and 8-bit
> variant names, e.g. %r10d (32), %r10w (16), and (I'm not sure these last
> ones really exist) %r10[hl] (8).

I put a little work into making the translator better handle signs/sizes
on registers and constants, but I didn't prepare it fully for these
cases you mention.  I'll take a look at that tomorrow.

Josh