Project

General

Profile

Bug #5876

sys/regset.h pollutes name space

Added by Gordon Ross over 4 years ago. Updated about 4 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Gordon Ross
Category:
-
Start date:
2015-04-29
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:
needs-triage

Description

Running a compile with -D__EXTENSIONS__ and this program:


/*
 * Compile on OpenIndiana with:
 * /opt/gcc/4.4.4/bin/gcc -std=c99 -c namespace-pollution.c
 * namespace-pollution.c:12:2: error: #error "ERR pollutes our namespace" 
 */

#define    _XOPEN_SOURCE 600
#define    __EXTENSIONS__ 1

#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#include <ctype.h>
#include <limits.h>

#ifdef    ERR
#error "ERR pollutes our namespace" 
#endif

int x;

History

#1

Updated by Gordon Ross over 4 years ago

One could easily argue that "With __EXTENSIONS__ defined, we can pollute as much as we want."
but there are quite a few interesting things applications may want to use that are only exposed when __EXTENSIONS__ is defined, so we really should come up with a way to pollute less here, at least when sys/regset.h was not specifically asked for.

#2

Updated by Gordon Ross over 4 years ago

I temporarily made sys/regset.h unreadable. Here's how we're getting it:

In file included from /usr/include/sys/signal.h:245,
                 from /usr/include/sys/procset.h:42,
                 from /usr/include/sys/wait.h:43,
                 from /usr/include/stdlib.h:40,
                 from namespace-pollution.c:13:
/usr/include/sys/ucontext.h:36:24: error: /usr/include/sys/regset.h: Permission denied

#3

Updated by Gordon Ross over 4 years ago

Here's a Samba bug referring to this problem:
https://bugzilla.samba.org/attachment.cgi?id=10623&action=edit

#4

Updated by Gordon Ross over 4 years ago

  • Subject changed from intel sys/regset.h pollutes name space to sys/regset.h pollutes name space
#5

Updated by Electric Monk over 4 years ago

  • Status changed from New to Closed
  • % Done changed from 0 to 100

git commit 21227944c2bcc086121a5428f3f9d2496ba646f5

commit  21227944c2bcc086121a5428f3f9d2496ba646f5
Author: Gordon Ross <gwr@nexenta.com>
Date:   2015-10-17T12:33:25.000Z

    5876 sys/regset.h pollutes name space
    Reviewed by: Josef 'Jeff' Sipek <jeffpc@josefsipek.net>
    Reviewed by: Jonathan Perkin <jperkin@joyent.com>
    Reviewed by: Alexander Pyhalov <alp@sfedu.ru>
    Approved by: Albert Lee <trisk@omniti.com>

#6

Updated by Gordon Ross over 4 years ago

  • % Done changed from 100 to 0

After 5876, the header file sys/regset.h is no longer
included by sys/ucontext.h, and therefore no longer
indirectly included by sys/signal.h, stdlib.h etc.

Most programs are not affected at all. Many programs
no longer need to work around "surprise" defines for
SS, CS, ERR, and the like.

A few programs will require changes as a result of
this header change, where the code expected symbols
like EAX to be defined after include <sys/ucontex.h>.
Such program are easily adapted to this change by
adding an include <sys/regset.h>. Without that,
the code will fail to compile with errors like:

  "signal.c", line 79: undefined symbol: UESP
  "signal.c", line 80: undefined symbol: EIP
  "signal.c", line 81: undefined symbol: EFL
  "signal.c", line 82: undefined symbol: EAX
  "signal.c", line 83: undefined symbol: EDX

There were several examples in illumos-gate where
this change was required. Here's one example:

--- a/usr/src/cmd/csh/i386/signal.c
+++ b/usr/src/cmd/csh/i386/signal.c
@@ -28,6 +26,7 @@
 #include <sys/types.h>
 #include <sys/siginfo.h>
 #include <sys/ucontext.h>
+#include <sys/regset.h>
 #include <signal.h>
 #include "signal.h" 
 #include <errno.h>

The new rule is basically: If your source module needs the register
name defines (EIP etc.) it needs to include <sys/regset.h>.

I hope this solves more problems than it causes.

#7

Updated by Gordon Ross over 4 years ago

  • Status changed from Closed to Feedback
  • Assignee set to Gordon Ross

It turns out there are many examples of external C code that expects
the register index defines after a #include <sys/ucontext.h> so we've
decided to just back this out for the moment. Sorry about the noise
and trouble.

I'll probably try an alternative approach that might hopefully meet both of these constraints:
(a) Provide the regset index definitions to code that includes <sys/ucontext.h>
(b) Do not expose those definitions to code that only included <sys/signal.h> etc.

#8

Updated by Gordon Ross about 4 years ago

Here's another fix attempt, this time maintaining compatibility for applications that include <ucontext.h>
https://github.com/gwr/illumos-gate/compare/2a44663...regset5d

#9

Updated by Electric Monk about 4 years ago

  • Status changed from Feedback to Closed
  • % Done changed from 0 to 100

git commit bc0e91320069f0bcaee43e80a7ea686d9efa2d08

commit  bc0e91320069f0bcaee43e80a7ea686d9efa2d08
Author: Gordon Ross <gwr@nexenta.com>
Date:   2015-12-08T01:47:26.000Z

    5876 sys/regset.h pollutes name space (try 2)
    Reviewed by: Josef 'Jeff' Sipek <jeffpc@josefsipek.net>
    Reviewed by: Jonathan Perkin <jperkin@joyent.com>
    Reviewed by: Alexander Pyhalov <alp@sfedu.ru>
    Reviewed by: Garrett D'Amore <garrett@damore.org>
    Approved by: Dan McDonald <danmcd@omniti.com>

Also available in: Atom PDF