Bug 666660 - inadequate parenthesization in systemtap probe macros
Summary: inadequate parenthesization in systemtap probe macros
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: systemtap
Version: 13
Hardware: Unspecified
OS: Unspecified
low
medium
Target Milestone: ---
Assignee: Frank Ch. Eigler
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2011-01-02 00:45 UTC by Tom Lane
Modified: 2013-07-03 03:34 UTC (History)
5 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2011-04-24 15:09:10 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description Tom Lane 2011-01-02 00:45:10 UTC
Description of problem:
Source code like this:
	TRACE_POSTGRESQL_SORT_DONE(state->tapeset != NULL, spaceUsed);
produces this warning:
        tuplesort.c:843: warning: comparison between pointer and integer
Investigation shows that the macro is expanding to
        do { size_t arg1 = (size_t)state->tapeset != ((void *)0); ...
so obviously the problem is failure to parenthesize the macro argument.

In this particular case it manages to give the right runtime behavior anyway,
but in other cases it would yield an outright wrong value for the probe
argument.

Version-Release number of selected component (if applicable):
systemtap-sdt-devel-1.3-3.fc13.x86_64

Additional info:
The probe definition is
	probe sort__done(bool, long);
though I'm not sure that matters.

Comment 1 Frank Ch. Eigler 2011-01-03 21:08:12 UTC
Can you check whether changing /usr/include/sys/sdt.h
by adding parentheses in these two places works for you?

67: #define STAP_CAST(t) (t)
83: #define STAP_CAST(t) (size_t)(t)

Stan, can you audit git systemtap's sdt.h also, to make sure
that incoming parameter expressions are parenthesized?  The
_SDT_PROBE() calls in #define STAP_PROBE... near line 200+
appear to lack this.

Comment 2 Tom Lane 2011-01-03 21:29:11 UTC
I confirm those changes silence the warning in Postgres.  Personally I'd spell the latter macro as "((size_t) (t))", but maybe I'm just overly paranoid.

Comment 3 Frank Ch. Eigler 2011-04-24 15:09:10 UTC
systemtap-1.4 corrects this.


Note You need to log in before you can comment on or make changes to this bug.