Re: Type punning question
* James Kanze:
On Jun 17, 9:49 pm, "Alf P. Steinbach" <al...@start.no> wrote:
* Travis Vitek:
I'm maintaining some code that uses the SunOS priocntl
system call [*], and this has led me to some questions about
type punning. To avoid a long-winded explaination, I'm just
going to post some source and then ask my questions.
Why are you posting in a C++ group?
This is C code.
I've got a lot of similar stuff in my C++. (I code for Posix
based machines, too.)
You can save quite a bit of work by simply not adding the complexities of the
lowest abstraction level possible. ;-)
#define PC_CLNMSZ 16
#define PC_CLINFOSZ (32 / sizeof (int))
// The pc_clinfo member is used to return data describing the
// attributes of a specific class. The format of this data is
// class-specific and is described under the appropriate
// heading.
typedef struct pcinfo
{
int pc_cid; /* class id */
char pc_clname[PC_CLNMSZ]; /* class name */
int pc_clinfo[PC_CLINFOSZ]; /* class information */
} pcinfo_t;
// realtime class attributes in the pc_clinfo buffer are in
// this format.
typedef struct rtinfo
{
short rt_maxpri;
} rtinfo_t;
Just a note, but all of the preceding has in fact been cribbed
from the system header, in order to present the problem. It's
not his code.
Yes.
int main ()
{
pcinfo_t pc;
// assume a call to priocntl() will initialize pc with
// something like this...
// pc.pc_clname [0] = 'R';
// pc.pc_clname [1] = 'T';
// pc.pc_clname [2] = 0;
//if (-1L == priocntl (idtype_t(), id_t(), PC_GETCLINFO, &pc))
// return -1;
//
rtinfo_t* prt = (rtinfo_t*)pc.pc_clinfo;
return 0 == prt->rt_maxpri;
}
The gcc-4.1.1 compiler generates the following warning when
casting pc.pc_clinfo (an array of int) to rtinfo_t*.
t.cpp: In function 'int main()':
t.cpp:38: warning: dereferencing type-punned pointer will break
strict-aliasing rules
It is clear that we are type-punning the pointer, but I do
not see how this particular case is dangerous.
On some machines breaking aliasing rules can cause a hardware
exception, on some machines and OSes that exception is caught
by the OS and fixed but then amounting to inefficient access.
And on most, if not all machines, compilers will assume that
lvalues expressions with different types can't alias the same
memory, with certain exceptions (most notably: lvalues with char
or unsigned char).
I'm not sure your statement is completely correct. C99 reportedly has a strict
aliasing requirement, that lvalues of different types can't refer to the same
(or overlapping) location. AFAIK C++ doesn't.
The hardware considerations are fairly irrelevant here, however,
Depends on what you mean by "here".
For the concrete case, since the cast is OK, the hardware is irrelevant.
In discussing the significance of such a warning and dangers of using such
casts, it's very relevant whether the aliased pointer points to a type that may
have stricter alignment requirements than the original data, which is a hardware
issue (or virtual machine issue).
He's dealing with a system interface, and presumably, the people
who specified the system API have ensured that it doesn't
violate any hardware constraints. As for the aliasing issues:
the actual memory is only accessed as a rtinfo_t, never as the
int[] it was declared as. So there isn't any actual conflict:
in fact, the system function priocntl will have constructed an
rtinfo_t in the memory reserved by pc.pc_clinfo, and accessing
it as anything other than an rtinfo_t (or a char or an unsigned
char) will result in undefined behavior. (I won't even try to
defend the quality of the system API---it's a true horror. But
as a normal user, you can't do anything about it; you just have
to live with it.)
Yes. :-)
While some of these questions may be more appropriate on the
gcc list, I'm hoping to better understand how this is
supposed to work, not how it happens to work on one
implementation. My questions are as follows...
1. is this actually a bogus warning?
No.
Definitely yes.
Yes, I agree, as you'd seen if you'd read my immediate correction.
He's used a reinterpret_cast to tell the
compiler to shut up, that he knows more than the compiler here
(which exceptionally happens to be true). The whole reason
d'?tre of reinterpret_cast is to shut up warnings (and errors);
there's something almost perverted for one to trigger a warning.
I suspect he may be compiling with C99 extensions enabled.
2. is this a dangerous thing to do?
Yes.
What he's doing is practically the cleanest and the safest thing
possible, given the interface he has to deal with.
In general it's quite dangerous.
And it's not at all clean.
On the contrary, the code deals with two differently typed views of the same
data, hence (the possibility of the silly-) the warning; as demonstrated, a
single view is possible and cleaner.
2. why does changing pc_clinfo to array of char eliminate the
warning? i.e., does that make the cast 'safe'?
Not sure why it eliminates the warning, although the fact that
you can convert any POD value to a sequence of chars and back
is probably involved in making the compiler not see the
problem. It doesn't make the cast safe.
It eliminates the warning, because the compiler doesn't see a
possible aliasing that it doesn't take into consideration. The
standard requires compilers to consider possible aliasing when
char and unsigned char are involved.
I suspect that you're trying to describe the correct explanation, but I don't
quite understand it (that is, I don't quite understand what's going on in the
compiler's "mind" here, in order to produce or not produce the warning).
Potentially, of course, it could result in the code not working,
since char normally has less restrictive alignment requirements
than int. If the preceding char array had an odd size, for
example, using the results of the cast could result in a core
dump. (Of course, the authors of the API ensured that the
preceding array didn't have an odd size.)
3. what techniques are there for avoiding problems like this?
If my understanding of the standard is correct, the above
does indeed invoke undefined behavior, but I'm not certain
how exactly to best work around the issue and to avoid
dangerous punning in the future.
C++ code would go like this (off the cuff) -- look ma, no casts! :-)
struct RtInfo
{
short maxPriority;
char filler[32 - sizeof(short)];
};
struct PcInfo
{
int id;
char className[16];
RtInfo rtInfo;
};
int main ()
{
PcInfo pc;
// assume a call to priocntl() will initialize pc with
// something like this...
// pc.classname [0] = 'R';
// pc.classname [1] = 'T';
// pc.classname [2] = 0;
//if (-1L == priocntl (idtype_t(), id_t(), PC_GETCLINFO, &pc))
// return -1;
return (0 == pc.rtInfo.maxPriority);
}
Except, of course, that he can't redefine the types, since
they're part of a system API.
The above doesn't redefine the types.
It simply replaces a silly processing sequence
[type A -> call -> cast to B* -> type B]
that's so roundabout and redundant that it causes a warning, with
[type B -> call]
Note that the called routine is variadic (or at least is documented as such).
It's that simple: using a single type, the result type, instead of 2 different
types with casting to the result type. Avoiding the casting isn't the main
issue, though, for with some other routine there would perhaps have to be a cast
in the call. Avoiding two differently typed views of the same data is an issue.
And the whole point of the
interface is that the real type of the data in the pc_clinfo
field depends on the preceding data (pc_cid and pc_clname).
pc_clinfo is basically a buffer (declared int to ensure adequate
alignment), in which the function priocntl "constructs" an
object whose type depends on other information in the structure.
Yes, the API design is sh*tty. :-)
Conceptually, the system request is returning a polymorphic
object; if all of the code were in C++, including the system API
(and we had garbage collection---not just over the process, but
at the system level, accross system and process boundaries), the
function would return a pointer to a base type, and he'd use
dynamic_cast to get at the additional data in the derived type.
No.
With a proper design there'd be separate routines with differently typed
arguments instead of a single routine with subfunctions chosen by an argument.
As the system API's (at least those of both Posix and Windows)
are defined in a very primitive C, however, such elegant options
aren't available, and we have to play such funny games.
No, that's no excuse; see right above.
If I
were writing a C++ wrapper for the function, I think I'd define
the class hierarchy (including a derived class for the error
cases), and return an std::auto_ptr to an instance of it. But I
don't expect to see anything that evolved in Posix or Windows
anytime soon.
IMHO, don't force unrelated types into a hierarchy.
Instead, I think it best to define each "logical" routine (sub-function) as a
distinct routine.
That allows static type checking.
PS: What *is* this thing/urge that programmers have about
introducing cryptic shortenings, like "hlt" instead of "halt",
A tradition from the PDP-11 days which said that names had to be
either three or six characters long.
Oh, thanks!
I didn't remember, and don't remember, but then I only did very little PDP-11
programming, in college.
Cheers,
- Alf
--
Due to hosting requirements I need visits to <url: http://alfps.izfree.com/>.
No ads, and there is some C++ stuff! :-) Just going there is good. Linking
to it is even better! Thanks in advance!