search

psmedley / gcc

GNU General Public License v2.0
7 stars 1 forks source link

Re-check _System calling convention behavior #3

Open dmik opened 10 years ago

dmik commented 10 years ago

In 2e73649db617b1778b3a96bd5db2911ab807d4dc (with fixes in 2049c7058166b1434d85ab0672c5374bde1041c3 and 411dfe140f03490ecf1dee2b24dcd720738dfeb7) I made _System behave as if extern "C" was also specified: i.e. suppress C++ name mangling.

As turned out here, https://github.com/bitwiseworks/mozilla-os2/issues/9#issuecomment-24843029, this has a side effect: if you declare a _System function within a namespace, it will actually be ignored and the function will get a global C-style name. In case if a function system with the same name is also defined in a different namespace, this will create a name conflict not expected by the programmer. This may eventually lead to unexpected behavior (as described in the link above).

The same will probably apply to situations where _System is used on class members (the class is also a namespace).

The current behavior is therefore wrong and should be fixed somehow. I need to check how IBM C/C++ behaves here and how GCC reacts on things like extern "C" inside C++ namespaces.

dmik commented 10 years ago

A quick thought is that we should suppress magical _System behavior that turns it into extern "C" name-wise, when the function is a member of a namespace or class. After all, most likely, this is done in IBM C so that these functions can be easily exported from DLLs by name in pure C style (i.e. old good API calls) but such old-style API functions should definitely not be part of any namespace or class — this simply doesn't make sense.

dmik commented 10 years ago

I've committed a workaround for Mozilla here: https://github.com/bitwiseworks/mozilla-os2/commit/7ec8faf4f28b870c5691a3fac74c053c6303cd35. Once the problem is fixed, this commit may be reverted (if necessary).

abwillis commented 10 years ago

Why is _System a problem with comm-esr17 but not in comm-esr10 for _write and friends even built with GCC 4.7.3? Or has the _System changes not been added to Paul's build? Prefacing with NPN I suppose breaks existing plugins (not that there are many to worry about)?

dmik commented 10 years ago

Andy, don't worry. These functions are the internal implementation of the Netscape Plugin interface used by XUL to map from C to its internal C++. They are not seen outside.

I think that it worked before because a) this code was not part of ESR10 and/or b) my patch was not part of the GCC build used by Dave to build it. Now it is (since long actually).

abwillis commented 10 years ago

a) It is part of the code, I checked that already. b) I built (and am posting from said build) using GCC 4.7.3 from earlier this month.

dmik commented 10 years ago

Okay, then this should be true: no code called _write() before. Now the chromium code does.

dmik commented 10 years ago

To clarify, by "internal" I mean not visible outside. But supplied to the plugins that expect _System so we can't just drop it.

dmik commented 10 years ago

There is also #9 which is related.