Caveat with lua_pushlightuserdata(L, (void *)&key)

classic Classic list List threaded Threaded
19 messages Options
Reply | Threaded
Open this post in threaded view
|

Caveat with lua_pushlightuserdata(L, (void *)&key)

Mike Pall-62
Hi,

the standard practice of using the address of a static const
object as a unique lightuserdata key is problematic:

static const int regkey_foo = 1;
static const int regkey_bar = 1;
...
lua_pushlightuserdata(L, (void *)&regkey_foo);
lua_pushlightuserdata(L, (void *)&regkey_bar);

PIL 27.3 describes this technique as "bulletproof" to get a
unique address. Alas, it's not.

The problem is that the compiler/linker may decide to join
constants with identical _values_. Unfortunately this makes the
_addresses_ the same, too. This in turn means the lightuserdata
keys are the same and happily overwrite each other in the
registry. :-(

Note that the linker may even decide to join constants from
different source files, linked to the same executable or shared
library.

[This is not a theory: LuaJIT 1.1.1 was broken on Mac OS X due to
this issue (GCC 4.0 + Mach linker).]

Solution:
- Ensure the const _values_ are unique across your whole program.
- Or use the addresses of non-const static variables.

Better check your programs now -- this is very hard to diagnose.

Bye,
     Mike

Reply | Threaded
Open this post in threaded view
|

Re: Caveat with lua_pushlightuserdata(L, (void *)&key)

Julien Cugnière
2006/6/24, Mike Pall <[hidden email]>:
static const int regkey_foo = 1;
static const int regkey_bar = 1;
[...]
The problem is that the compiler/linker may decide to join
constants with identical _values_. Unfortunately this makes the
_addresses_ the same, too.

Wow, that's interesting.

Solution:
- Or use the addresses of non-const static variables.

Hmm, pushing the idea further, couldn't a smart optimizing compiler
automatically constify non-const variables that the code does not
modify ? You would then have to use volatile non-const static
variables...

Or is the compiler able to make any assumptions about constness once
you take the address of a variable and cast it to (void*) ?

Anyway, here is a simple way to ensure that your constants are unique
(assuming this legal C):

   static const void* regkey1 = &regkey1;
   static const void* regkey2 = &regkey2;

--
Julien Cugnière


Reply | Threaded
Open this post in threaded view
|

RE: Caveat with lua_pushlightuserdata(L, (void *)&key)

Richard Ranft
In reply to this post by Mike Pall-62
This seems like the most annoying case of excessive optimization possible.
Why on Earth would you want this?  Two static const variables with the same
value could still represent different purposes - the compiler and linker
have no business optimizing out your purpose.

Thanks for the heads-up, though.  Talk about a gotcha waiting to happen....

Richard

> PIL 27.3 describes this technique as "bulletproof" to get a
> unique address. Alas, it's not.
>
> The problem is that the compiler/linker may decide to join
> constants with identical _values_. Unfortunately this makes the
> _addresses_ the same, too. This in turn means the lightuserdata
> keys are the same and happily overwrite each other in the
> registry. :-(
>
> Note that the linker may even decide to join constants from
> different source files, linked to the same executable or shared
> library.
>
> [This is not a theory: LuaJIT 1.1.1 was broken on Mac OS X due to
> this issue (GCC 4.0 + Mach linker).]


Reply | Threaded
Open this post in threaded view
|

Re: Caveat with lua_pushlightuserdata(L, (void *)&key)

Andreas Stenius-2
In reply to this post by Julien Cugnière
Julien Cugnière skrev:
Anyway, here is a simple way to ensure that your constants are unique
(assuming this legal C):

   static const void* regkey1 = &regkey1;
   static const void* regkey2 = &regkey2;


No, &regkey1 is not a constant expression, although the result is constant.

I would use the __FILE__ and __LINE__ macros to get unique values,
unless space is an issue.

Something like:

#define STRINGIFY(a)	#a
#define SAPPEND(s,i)	s "" STRINGIFY(i)
#define UNIQUE_KEY	SAPPEND(__FILE__, __FILE__)

static const char* unique_key = UNIQUE_KEY;

// just make sure not to have more than one key-macro per line


Regards,
	Andreas

Reply | Threaded
Open this post in threaded view
|

Re: Caveat with lua_pushlightuserdata(L, (void *)&key)

Mike Pall-62
In reply to this post by Richard Ranft
Hi,

Richard Ranft wrote:
> This seems like the most annoying case of excessive optimization possible.
> Why on Earth would you want this?  Two static const variables with the same
> value could still represent different purposes - the compiler and linker
> have no business optimizing out your purpose.

Sure, it's pretty pointless to do this for tiny constants. But it
seems to be a logical extension of string constant joining. And
many toolchains have been doing this for a long time (try
printf("%p\n", "foo") from two different source files linked to
the same executable -- you may need to compile with -O).

Maybe someone orthogonalized the linker a bit too much. I still
think it's not forbidden by the various C standards.

Bye,
     Mike

Reply | Threaded
Open this post in threaded view
|

Re: Caveat with lua_pushlightuserdata(L, (void *)&key)

David Jones-2
In reply to this post by Andreas Stenius-2

On 26 Jun 2006, at 09:33, Andreas Stenius wrote:

Julien Cugnière skrev:
Anyway, here is a simple way to ensure that your constants are unique
(assuming this legal C):
   static const void* regkey1 = &regkey1;
   static const void* regkey2 = &regkey2;

No, &regkey1 is not a constant expression, although the result is constant.

Doesn't matter. In C it's legal (though probably stupid) to take a pointer to a const object and convert it to a pointer to a non-const object. No idea about C++. Just one of the many ways in which const is broken and surprises programmers.

drj

Reply | Threaded
Open this post in threaded view
|

Re: Caveat with lua_pushlightuserdata(L, (void *)&key)

Roberto Ierusalimschy
In reply to this post by Mike Pall-62
> Maybe someone orthogonalized the linker a bit too much. I still
> think it's not forbidden by the various C standards.

I think it is. ANSI C99 says this about linkage:

  6.2.2 Linkages of identiïers

  1 - An identiïer declared in different scopes or in the same scope
  more than once can be made to refer to the same object or function by
  a process called linkage.(21)

  (21) THERE IS NO LINKAGE BETWEEN DIFFERENT IDENTIFIERS.

(my emphasis)

-- Roberto

Reply | Threaded
Open this post in threaded view
|

Re: Caveat with lua_pushlightuserdata(L, (void *)&key)

Glenn Maynard
In reply to this post by David Jones-2
On Mon, Jun 26, 2006 at 11:33:37AM +0100, David Jones wrote:
> 
> On 26 Jun 2006, at 09:33, Andreas Stenius wrote:
> 
> >Julien Cugnière skrev:
> >>Anyway, here is a simple way to ensure that your constants are unique
> >>(assuming this legal C):
> >>   static const void* regkey1 = &regkey1;
> >>   static const void* regkey2 = &regkey2;
> >
> >No, &regkey1 is not a constant expression, although the result is  
> >constant.
> 
> Doesn't matter.  In C it's legal (though probably stupid) to take a  
> pointer to a const object and convert it to a pointer to a non-const  
> object.  No idea about C++.  Just one of the many ways in which const  
> is broken and surprises programmers.

Without getting into an extended debate, I'd suggest that anyone reading
this and thinking about not using const get a better understanding of
it.  Remember that it's just another type safety mechanism; it's there
for programmers, not the compiler; like most type safety mechanisms, it
can be overridden, but be sure it makes sense to do so.  Don't expect
const to improve optimization; that's not what it's for.  (And always
compile your C programs in gcc with "-Wwrite-strings", so string constants
are const.  g++ does this by default.)

I'm not sure if the problem here is a bug in Apple's version of GCC
or just an incorrect assumption about pointer uniqueness; that said, I
wonder if simply using eg. "volatile const int i = 1;" would work around
the problem.  It would impact optimization of accessing "i", but since
only the pointer is used, that shouldn't matter; and it may tip the
compiler that it can't merge it with other variables.  If it's a
compiler bug, maybe there's a flag to disable it.

-- 
Glenn Maynard

Reply | Threaded
Open this post in threaded view
|

Re: Caveat with lua_pushlightuserdata(L, (void *)&key)

Roberto Ierusalimschy
> I'm not sure if the problem here is a bug in Apple's version of GCC
> or just an incorrect assumption about pointer uniqueness;

I think the C standard is clear about that (no linkage between different
identifiers). This is a bug in GCC.

-- Roberto

Reply | Threaded
Open this post in threaded view
|

Re: Caveat with lua_pushlightuserdata(L, (void *)&key)

Roberto Ierusalimschy
In reply to this post by Glenn Maynard
> I'm not sure if the problem here is a bug in Apple's version of GCC
> or just an incorrect assumption about pointer uniqueness;

I think the standard is clear about this point (no linkage between
different identifiers). This a bug in GCC.

-- Roberto

Reply | Threaded
Open this post in threaded view
|

Re: Caveat with lua_pushlightuserdata(L, (void *)&key)

Mike Pall-62
In reply to this post by Roberto Ierusalimschy
Hi,

Roberto Ierusalimschy wrote:
> I think the C standard is clear about that (no linkage between different
> identifiers). This is a bug in GCC.

I'm not so sure it's GCC. It doesn't happen here on Linux with
any GCC version I've tried (3.3, 3.4, 4.1).

The bug might be just in the specific (heavily-patched) GCC 4.0
Apple is using. Or only in combination with the Mach linker.

Here's a trivial test program:

  #include <stdio.h>

  static const int regkey_foo = 1;
  static const int regkey_bar = 1;

  int main(int argc, char **argv)
  {
    printf("%p %p\n", &regkey_foo, &regkey_bar);
    return 0;
  }

Note: It must be compiled with optimization on.

It _should_ print two different numbers. If it doesn't, then
(according to Roberto) the toolchain is non-standards compliant.
I've verified it's ok on various Linux, *BSD or Solaris hosts
with various GCC and linker versions and variants.

It would be interesting if someone with access to a few different
Mac OS X versions (PPC and/or Intel Macs) could try this. Please
include the OS, GCC and linker versions. Maybe try this with and
without optimization. Or try -std=c89 or -std=c99 to enforce a
particular standard and see whether it happens, too.

If a pattern emerges then we could file a bug report with either
Apple or the GCC team.

Bye,
     Mike

Reply | Threaded
Open this post in threaded view
|

Re: Caveat with lua_pushlightuserdata(L, (void *)&key)

Mike Pall-62
Hi,

I wrote:
> The bug might be just in the specific (heavily-patched) GCC 4.0
> Apple is using. Or only in combination with the Mach linker.

Ok, the problem has been identified! The culprit is the Mach-O
specific code emitter for GCC. It does not properly handle the
merge constants flag.

It confuses labelled constants and constant literals and puts
them into a section which is coalesced by the linker (in this
case: __TEXT,__literal4). Thus all const variables with the same
value get the same address (which violates the C/C++ standards).

The logic in gcc/config/darwin.c machopic_select_section() is
broken. The configuration variable "flag_merge_constants" has two
values: 1 (-fmerge-constants) is the default with optimization
and 2 (-fmerge-all-constants) is non-standards-compliant and NOT
enabled by default. But the code does not differentiate these two
cases and makes the wrong decisions.

The standard section selector code gets it right, of course (see
gcc/varasm.c categorize_decl_for_section()). The comments in the
code clearly indicate the potential problem, too:

| C and C++ don't allow different variables to share the same
| location.  -fmerge-all-constants allows even that (at the
| expense of not conforming).

I'm sorry, but I cannot contribute a patch because I don't know
enough about the internal GCC logic to get it right.

This is a long standing bug and affects all GCC 3.x and GCC 4.x
versions on Mac OS X (PPC and Intel Macs). The code for the
Mach-O code emitter has been contributed by Apple.

I'm not sure where to report this. If anyone has either close
ties to Apple or the GCC team -- please can you take over to
report it and track its progress? Thank you!

[Of course the test program can be used for the bug report.]

Thanks to Adam for helping me diagnose this problem.

Bye,
     Mike

Reply | Threaded
Open this post in threaded view
|

Re: Caveat with lua_pushlightuserdata(L, (void *)&key) [OT]

Glenn Maynard
In reply to this post by Glenn Maynard
My private reply to David bounced due to an SMTP configuration error on
his end.  I'm just forwarding it here so he's aware of it and might get
it fixed, or at least be aware that his server is rejecting legitimate
mail.  Sorry for the noise.

Reporting-MTA: dns; c-65-96-98-57.hsd1.ma.comcast.net
X-Postfix-Queue-ID: 55E90100A0855
X-Postfix-Sender: rfc822; [hidden email]
Arrival-Date: Tue, 27 Jun 2006 14:52:54 -0400 (EDT)

Final-Recipient: rfc822; [hidden email]
Action: failed
Diagnostic-Code: X-Postfix; host mx-all.pobox.com[208.58.1.198] said: 554
    <[hidden email]>: Recipient address rejected: broadband/ returned deny:
    c-65-96-98-57.hsd1.ma.comcast.net looks like a consumer broadband machine
    (contains 65.96.98.57) (in reply to RCPT TO command)

Date: Tue, 27 Jun 2006 14:52:54 -0400
From: Glenn Maynard <[hidden email]>
Subject: Re: Caveat with lua_pushlightuserdata(L, (void *)&key)
To: David Jones <[hidden email]>

-- 
Glenn Maynard


Reply | Threaded
Open this post in threaded view
|

Re: Caveat with lua_pushlightuserdata(L, (void *)&key) [OT]

Anders Bergh-2
I'm pretty sure that's not a configuration error, you shouldn't send
e-mail from your home connection. Most ISP's block those messages due
to spam.

On 6/27/06, Glenn Maynard <[hidden email]> wrote:
My private reply to David bounced due to an SMTP configuration error on
his end.  I'm just forwarding it here so he's aware of it and might get
it fixed, or at least be aware that his server is rejecting legitimate
mail.  Sorry for the noise.

Reporting-MTA: dns; c-65-96-98-57.hsd1.ma.comcast.net
X-Postfix-Queue-ID: 55E90100A0855
X-Postfix-Sender: rfc822; [hidden email]
Arrival-Date: Tue, 27 Jun 2006 14:52:54 -0400 (EDT)

Final-Recipient: rfc822; [hidden email]
Action: failed
Status: 5.0.0
Diagnostic-Code: X-Postfix; host mx-all.pobox.com[208.58.1.198] said: 554
    <[hidden email]>: Recipient address rejected: broadband/ returned deny:
    c-65-96-98-57.hsd1.ma.comcast.net looks like a consumer broadband machine
    (contains 65.96.98.57) (in reply to RCPT TO command)

Date: Tue, 27 Jun 2006 14:52:54 -0400
From: Glenn Maynard <[hidden email]>
Subject: Re: Caveat with lua_pushlightuserdata(L, (void *)&key)
To: David Jones <[hidden email]>

--
Glenn Maynard



--
Anders

Reply | Threaded
Open this post in threaded view
|

Re: Caveat with lua_pushlightuserdata(L, (void *)&key)

David Given
In reply to this post by Mike Pall-62
Mike Pall wrote:
[...]
> The bug might be just in the specific (heavily-patched) GCC 4.0
> Apple is using. Or only in combination with the Mach linker.

I did run into an OSX bug in the past where it wasn't zero-initialising global
variables:

#include <stdio.h>

int foo;

int main(int argc, char* argv[])
{
    printf("%d\n", foo);
}

This must return '0' (the standard requires it), but the Apple wasn't.
Explicitly initialising 'foo' with 'int foo = 0;' worked, but that shouldn't
be necessary.

This was back in the days of OSX on PowerPC; and I don't know whether the
bug's been fixed or not by now.

-- 
+- David Given --McQ-+ "Gaping from its single obling socket was
|  [hidden email]    | scintillating, many fauceted scarlet emerald..."
| ([hidden email]) | --- Jim Theis, _The Eye of Argon_ (spelling
+- www.cowlark.com --+ original)


Attachment: signature.asc
Description: OpenPGP digital signature

Reply | Threaded
Open this post in threaded view
|

Re: Caveat with lua_pushlightuserdata(L, (void *)&key)

Diego Nehab-3
Hi,

I did run into an OSX bug in the past where it wasn't zero-initialising global
variables:

#include <stdio.h>

int foo;

int main(int argc, char* argv[])
{
   printf("%d\n", foo);
}

This must return '0' (the standard requires it), but the Apple wasn't.
Explicitly initialising 'foo' with 'int foo = 0;' worked, but that shouldn't
be necessary.

This was back in the days of OSX on PowerPC; and I don't know whether the
bug's been fixed or not by now.

Don't you have to declare the variable static? Otherwise I
believe it's ok to leave the value uninitialized (at least in C).

Regards,
Diego.

Reply | Threaded
Open this post in threaded view
|

Re: Caveat with lua_pushlightuserdata(L, (void *)&key)

Raffaele Salmaso
In reply to this post by David Given
David Given ha scritto:
This was back in the days of OSX on PowerPC; and I don't know whether the
bug's been fixed or not by now.
00:37:07 Mer Giu 28 raf@jack ~
$ gcc -v
Using built-in specs.
Target: powerpc-apple-darwin8
Configured with: /private/var/tmp/gcc/gcc-5341.obj~1/src/configure --disable-checking -enable-werror --prefix=/usr --mandir=/share/man --enable-languages=c,objc,c++,obj-c++ --program-transform-name=/^[cg][^.-]*$/s/$/-4.0/ --with-gxx-include-dir=/include/c++/4.0.0 --with-slibdir=/usr/lib --build=powerpc-apple-darwin8 --host=powerpc-apple-darwin8 --target=powerpc-apple-darwin8
Thread model: posix
gcc version 4.0.1 (Apple Computer, Inc. build 5341)
00:37:29 Mer Giu 28 raf@jack ~
$ cat prova.c
#include <stdio.h>

int foo;

int main(int argc, char* argv[])
{
        printf("%d\n", foo);
}

00:37:32 Mer Giu 28 raf@jack ~
$ ./prova
0
00:37:35 Mer Giu 28 raf@jack ~
$ uname -a
Darwin jack.local 8.7.0 Darwin Kernel Version 8.7.0: Fri May 26 15:20:53 PDT 2006; root:xnu-792.6.76.obj~1/RELEASE_PPC Power Macintosh powerpc

so it seems to be fixed

--
()_() | NN KAPISCO XK' CELLHAVETE T'ANNTO CN ME SL         | +----
(o.o) | XK' SKRIVO 1 P'HO VELLOCE MA HALL'ORA DITTELO      | +---+
'm m' | KE SIETE VOI K CI HAVVETE PROBBLEMI NO PENSATECI   |  O  |
(___) | HE SENZA RANKORI CIAOOOO                           |
                   raffaele punto salmaso at gmail punto com

Reply | Threaded
Open this post in threaded view
|

Re: Caveat with lua_pushlightuserdata(L, (void *)&key)

Raffaele Salmaso
In reply to this post by Diego Nehab-3
Diego Nehab ha scritto:
Don't you have to declare the variable static? Otherwise I
believe it's ok to leave the value uninitialized (at least in C).
oh, yes
it's C++ with static default, not C

--
()_() | NN KAPISCO XK' CELLHAVETE T'ANNTO CN ME SL         | +----
(o.o) | XK' SKRIVO 1 P'HO VELLOCE MA HALL'ORA DITTELO      | +---+
'm m' | KE SIETE VOI K CI HAVVETE PROBBLEMI NO PENSATECI   |  O  |
(___) | HE SENZA RANKORI CIAOOOO                           |
                   raffaele punto salmaso at gmail punto com

Reply | Threaded
Open this post in threaded view
|

Re: Caveat with lua_pushlightuserdata(L, (void *)&key)

Glenn Maynard
In reply to this post by Diego Nehab-3
On Tue, Jun 27, 2006 at 06:35:56PM -0400, Diego Nehab wrote:
> Don't you have to declare the variable static? Otherwise I
> believe it's ok to leave the value uninitialized (at least in C).

Not in a global scope; global variables are always initialized.
You're thinking of a local scope, where static causes the variable
to not be stored on the stack.

-- 
Glenn Maynard