RFC: GCC plugin to find encrypted function pointer calls in glibc

RFC: GCC plugin to find encrypted function pointer calls in glibc

[Aldy Hernandez]

[-- Attachment #1: Type: text/plain, Size: 2288 bytes --]

Hi folks!

I'm working on a GCC plugin to pinpoint places in glibc where indirect 
function calls are made through a function pointer that has not been 
demangled.

As a reference, I am attaching a test case with various things that my 
plugin currently warns on that may serve as examples to what I am trying 
to accomplish.

The general idea is that I have defined a decryption operation as the 
output of a binary operator (xor, ior, and), so while we would warn for 
something like this:

	typedef void (*callback_t) (void);
	void foo (callback_t cb)
	{
	  cb ();
	}

...the following would be OK:

	#define PTR_DEMANGLE(var) \
	  (var) = (__typeof(var)) ((unsigned long) (var) ^ MAGIC)

	typedef void (*callback_t) (void);
	void foo (callback_t cb)
	{
	  callback_t tmp = cb;
	  PTR_DEMANGLE (tmp);
	  tmp ();
	}

However, I see glibc uses the following idiom:

#  define PTR_DEMANGLE(reg)	asm ("ror $2*" LP_SIZE "+1....)

Since I would prefer not to assume the output of all inline asm's are a 
demangling operation, I would like to get feedback from the community on 
what would be preferred.

My preferred approach is to add an attribute to an inline function that 
would wrap the asm:

	__attribute__((decrypt)) static inline funcp demangler (funcp f)
	{
		asm("blah");
	}

This is straightforward, clean, and follows language semantics (not to 
mention that I already have it implemented into my plugin :)), but 
Florian made funny faces when I showed it to him, so here I am :).

It would be neat if GCC had a way of tagging individual gimple 
statements with an attribute, so we could tag them with 
__attribute__((decrypt)), but alas we don't have such mechanism, and I'd 
prefer not to perform major surgery to GCC to make it so.

Another alternative would be to tag inline asms with commented out magic 
at the end, such that the plugin would notice and take appropriate action:

#ifdef FUNCTION_POINTER_CHECKS
#define DEMANGLE_TAG " ##_DEMANGLE_##"
#else
#define DEMANGLE_TAG ""
#endif
#  define PTR_DEMANGLE(var)  asm("ror $2*..."##DEMANGLE_TAG \
				: "=r" (var)
				: "0" (var)
				etc
				etc

This is straightforward, but I still prefer the inline function plus 
attribute idea.

If anyone is interested, I can post the code to my plugin.

What do y'all think?

Aldy

[-- Attachment #2: test-funcp.c --]
[-- Type: text/plain, Size: 2947 bytes --]

typedef void (*funcp) (void);

/* Binary operations on a function pointer is a recognized form of
   pointer encryption.

   The decryption must be done in place (to a local/temporary or a
   register), and never written to memory, otherwise it is considered
   unsafe.  */
#define PTR_DEMANGLE(var) \
  (var) = (__typeof(var)) ((unsigned long) (var) ^ 0xc00ffee)

/* A function with __attribute__((encrypt)) is considered an
   acceptable way to encrypt a function pointer.  Similar to PTR_DEMANGLE
   above, the decryption must be done in place.

   Note: It would be neat if we could tag the attribute directly onto the
   asm statement, but GCC currently doesn't do this, and it would
   probably be over kill.  */
__attribute__((encrypt))
static funcp asm_demangler (funcp f)
{
  asm ("#decrypt_operation $2, $0" : "=r" (f) : "0" (f));
  return f;
}

void (*encrypted_funcp) (void);
void (*plain_funcp) (void);
void direct_function (void);

extern int bar();

void foo()
{
  PTR_DEMANGLE (encrypted_funcp);
  encrypted_funcp ();		// WARN, written to memory.

  encrypted_funcp = asm_demangler (encrypted_funcp);
  encrypted_funcp ();		// WARN, written to memory.

  plain_funcp ();		// WARN, no decryption at all.

  direct_function();		// OK.  Don't care about plain func calls.

  funcp f = encrypted_funcp;
  f = asm_demangler (f);
  f();				// OK.  Decryption to a temporary, not memory.

  f = encrypted_funcp;
  f();				// WARN, no decryption at all.

  PTR_DEMANGLE (f);
  f();				// OK. Decryption in place.

  f = encrypted_funcp;
  if (bar())
    PTR_DEMANGLE (f);
  f ();				// WARN, path not demangled.

  // Test that phi convergence code works.
  f = encrypted_funcp;
  if (bar())
    PTR_DEMANGLE (f);
  else
    {
      funcp g = f;
      PTR_DEMANGLE (g);
      f = g;
    }
  f ();				// OK, all paths demangled.

  f = encrypted_funcp;
  if (bar())
    PTR_DEMANGLE (f);
  f();				// WARN, only one path mangled.

  extern void qsort(funcp callback);
  qsort (encrypted_funcp); // WARN, passing non decrypted funcp in argument

  qsort(0);		    // OK to pass a constant even though it's
			    // not decrypted.

  if (encrypted_funcp == 0) // WARN, comparing non-decrypted with constant.
    direct_function();

  f = encrypted_funcp;
  PTR_DEMANGLE (f);
  if (f != 0)		    // OK to compare with constant, we've been
			    // demangled.
    direct_function();
}

void foo2()
{
  funcp f = encrypted_funcp;
  funcp g = plain_funcp;
  PTR_DEMANGLE (f);
  if (f == g)		   // WARN, incompatible encryptness comparison
    bar();

  f = encrypted_funcp;     // OK to compare same encryption type.
  if (f == g)
    bar();
}

// Test that the propagator doesn't go into an infinite loop.

void
bad_inifinite_loop (const signed char *inptr)
{
  while (1)
    {
      signed char ch = *inptr;

      if (!ch)
        continue;

      ch &= 0x1f;

      for (unsigned i = 1; i < 10000; ++i)
        ch |= inptr[0] & 0x3f;
    }
}

Re: RFC: GCC plugin to find encrypted function pointer calls in glibc

[Carlos O'Donell]

On 04/29/2016 06:23 AM, Aldy Hernandez wrote:
> Since I would prefer not to assume the output of all inline asm's are
> a demangling operation, I would like to get feedback from the
> community on what would be preferred.

Awesome looking work!
 
> My preferred approach is to add an attribute to an inline function that would wrap the asm:
> 
>     __attribute__((decrypt)) static inline funcp demangler (funcp f)
>     {
>         asm("blah");
>     }
> 
> This is straightforward, clean, and follows language semantics (not
> to mention that I already have it implemented into my plugin :)), but
> Florian made funny faces when I showed it to him, so here I am :).

You can build glibc with gcc 4.7 or newer.

(1) static inline wrapper with function attribute:

For a representative set of architectures, say x86_64, i686, ppc64,
s390x, aarch64, and arm, does a 4.7 or newer compiler do as good a
job with a static inline function as it does with the macro that
evaluates to a bare assembly?

(2) Asm attribute:

We have function, variable, type, label, and enumerator attributes,
why not support them on asm statements to mark them as decrypt or
encrypt functions? This would lend itself to a more natural conversion
of inline assembly that users may want to write and embed into their
programs.

It is entirely conceivable that (1) does a good-enough job, and we
add (2) at the same time to the latest gcc and backport. When we raise
the supported build compiler version we can switch to (2) if needed.

-- 
Cheers,
Carlos.

Re: RFC: GCC plugin to find encrypted function pointer calls in glibc

[Aldy Hernandez]

On 04/29/2016 09:21 PM, Carlos O'Donell wrote:
> On 04/29/2016 06:23 AM, Aldy Hernandez wrote:
>> Since I would prefer not to assume the output of all inline asm's are
>> a demangling operation, I would like to get feedback from the
>> community on what would be preferred.
>
> Awesome looking work!
>
>> My preferred approach is to add an attribute to an inline function that would wrap the asm:
>>
>>      __attribute__((decrypt)) static inline funcp demangler (funcp f)
>>      {
>>          asm("blah");
>>      }
>>
>> This is straightforward, clean, and follows language semantics (not
>> to mention that I already have it implemented into my plugin :)), but
>> Florian made funny faces when I showed it to him, so here I am :).
>
> You can build glibc with gcc 4.7 or newer.
>
> (1) static inline wrapper with function attribute:
>
> For a representative set of architectures, say x86_64, i686, ppc64,
> s390x, aarch64, and arm, does a 4.7 or newer compiler do as good a
> job with a static inline function as it does with the macro that
> evaluates to a bare assembly?

This requires some careful analysis.  I will look into this next week 
and report back.

>
> (2) Asm attribute:
>
> We have function, variable, type, label, and enumerator attributes,
> why not support them on asm statements to mark them as decrypt or
> encrypt functions? This would lend itself to a more natural conversion
> of inline assembly that users may want to write and embed into their
> programs.

I haven't looked deeply into this, but here are my initial thoughts in 
the interest of not stalling the conversation.

All of the things you mention are types (sort of).  GCC has support for 
attributes on types.  We don't support attributes on statements, which 
have an entirely different representation (gimple).  I would be hesitant 
to add attributes to statements, just for supporting one plugin.  Plus 
the fact that unless we revamp the way attributes work in GCC 
altogether, we'll end up having to strcmp() our way through the 
attribute list in each pass that is interested in attributes (for every 
single interesting statement).

So... (a) inefficient in its current state, (b) involved to get it 
working efficiently, yet only for one plugin.  And that one plugin could 
conceivably use the present infrastructure (inline functions).

That being said, I owe you some analysis of what GCC 4.7 does with 
inline asm's.

>
> It is entirely conceivable that (1) does a good-enough job, and we
> add (2) at the same time to the latest gcc and backport. When we raise
> the supported build compiler version we can switch to (2) if needed.
>

I take it option 3 is too disgusting, provided #1 is a no go?  That is, 
adding a carefully crafted comment at the end of an inline asm string?

Errr, and by the way, is it a requirement that this plugin work with 
4.7?  Because, we only need this for static analysis.  Is building glibc 
twice a no go (once with the plugin for static analysis, and once for 
building the actual production code)?  I thought the purpose of the 
plugin was just to analyze what's already there, but I do see the 
benefit of using the same compiler for static analysis _and_ for 
building the library.

If the same compiler is a requirement, the plugin requires some 
rewriting, because unfortunately, the plugin infrastructure is a moving 
API target.  I doubt a plugin for GCC 6 will even compile the boiler 
plate nonsense we need for another GCC version (and vice versa).

Aldy

Re: RFC: GCC plugin to find encrypted function pointer calls in glibc

[Florian Weimer]

On 04/30/2016 03:21 AM, Carlos O'Donell wrote:

> (1) static inline wrapper with function attribute:
>
> For a representative set of architectures, say x86_64, i686, ppc64,
> s390x, aarch64, and arm, does a 4.7 or newer compiler do as good a
> job with a static inline function as it does with the macro that
> evaluates to a bare assembly?

We need both because the inline function cannot be type-generic.  There 
would have to be a macro wrapping the inline function, with an 
appropriate cast to preserve the function pointer type.

Florian

Re: RFC: GCC plugin to find encrypted function pointer calls in glibc

[Aldy Hernandez]

On 04/30/2016 12:12 PM, Florian Weimer wrote:
> On 04/30/2016 03:21 AM, Carlos O'Donell wrote:
>
>> (1) static inline wrapper with function attribute:
>>
>> For a representative set of architectures, say x86_64, i686, ppc64,
>> s390x, aarch64, and arm, does a 4.7 or newer compiler do as good a
>> job with a static inline function as it does with the macro that
>> evaluates to a bare assembly?
>
> We need both because the inline function cannot be type-generic.  There
> would have to be a macro wrapping the inline function, with an
> appropriate cast to preserve the function pointer type.

Correct, but it's localized in one file per architecture, right?  So, 
not a big problem?

Re: RFC: GCC plugin to find encrypted function pointer calls in glibc

[Florian Weimer]

On 04/30/2016 06:20 PM, Aldy Hernandez wrote:
> On 04/30/2016 12:12 PM, Florian Weimer wrote:
>> On 04/30/2016 03:21 AM, Carlos O'Donell wrote:
>>
>>> (1) static inline wrapper with function attribute:
>>>
>>> For a representative set of architectures, say x86_64, i686, ppc64,
>>> s390x, aarch64, and arm, does a 4.7 or newer compiler do as good a
>>> job with a static inline function as it does with the macro that
>>> evaluates to a bare assembly?
>>
>> We need both because the inline function cannot be type-generic.  There
>> would have to be a macro wrapping the inline function, with an
>> appropriate cast to preserve the function pointer type.
>
> Correct, but it's localized in one file per architecture, right?  So,
> not a big problem?

Agreed, I don't see a problem with that.  Just the inline function would 
have to be architecture-specific.  These days, we probably don't even 
need custom inline assembly anymore (just the architecture-specified way 
to access thread-local state).

Florian

Re: RFC: GCC plugin to find encrypted function pointer calls in glibc

[Aldy Hernandez]

On 04/29/2016 09:21 PM, Carlos O'Donell wrote:

> You can build glibc with gcc 4.7 or newer.
>
> (1) static inline wrapper with function attribute:
>
> For a representative set of architectures, say x86_64, i686, ppc64,
> s390x, aarch64, and arm, does a 4.7 or newer compiler do as good a
> job with a static inline function as it does with the macro that
> evaluates to a bare assembly?

I grabbed the relevant bits from various glibc backends (x86_64, ppc64, 
and arm) and made a testcase that would work on all three architectures. 
  All we need to verify is that the code generated for 
using_original_macro_demangler() and using_new_inline_demangler() is 
exactly the same for all three architectures.  I have visually verified 
that this is so.

It looks like all other architectures are variants of the same pattern 
(inline asm or some boiler plate code looking at a global that may be in 
a TLS variable), so I didn't bother.

As you can see, what I propose is something as simple as:

#define OLD_MACRO_PTR_DEMANGLE(var) asm("magic")

static inline uintptr_t
new_ptr_demangle (uintptr_t var)
{
   OLD_MACRO_PTR_DEMANGLE (var);
   return var;
}

#define PTR_DEMANGLE(var) \
   var = (typeof(var)) new_ptr_demangle ((uintptr_t) var)

Do y'all agree?  Can we use inline functions wrapping the asm's and tag 
an attribute on the function?

Aldy


/* Testcase to compare pointer demangling code on various
    architectures.  */

typedef struct
{
   int blah1;
   int blah2;
   unsigned int pointer_guard;
   int blah3;
} tcbhead_t;

#ifdef __x86_64__
typedef unsigned long long int uintptr_t;
#define LP_SIZE "8"
#  define PTR_DEMANGLE(var)	asm ("ror $2*" LP_SIZE "+1, %0\n"	\
				     "xor %%fs:%c2, %0"			\
				     : "=r" (var)			\
				     : "0" (var),			\
				       "i" (__builtin_offsetof (tcbhead_t,    \
						      pointer_guard)))
#elif defined __powerpc64__
typedef unsigned long long int uintptr_t;
register void *__thread_register __asm__ ("r13");
# define TLS_TCB_OFFSET 0x7000
# define THREAD_GET_POINTER_GUARD() \
     (((tcbhead_t *) ((char *) __thread_register 
        \
                      - TLS_TCB_OFFSET))[-1].pointer_guard)
#  define PTR_MANGLE(var) \
   (var) = (__typeof (var)) ((uintptr_t) (var) ^ 
THREAD_GET_POINTER_GUARD ())
#  define PTR_DEMANGLE(var)	PTR_MANGLE (var)

#elif defined __arm__
typedef unsigned long int uintptr_t;
# define attribute_hidden __attribute__ ((visibility ("hidden")))
#define attribute_relro __attribute__ ((section (".data.rel.ro")))
/* Only exported for architectures that don't store the pointer guard
    value in thread local area.  */
uintptr_t __pointer_chk_guard
         attribute_relro attribute_hidden __attribute__ ((nocommon));
#  define PTR_MANGLE(var) \
   (var) = (__typeof (var)) ((uintptr_t) (var) ^ __pointer_chk_guard)
#  define PTR_DEMANGLE(var)     PTR_MANGLE (var)
#else
#error "architecture not supported"
#endif

static inline uintptr_t
ptr_demangle (uintptr_t var)
{
   PTR_DEMANGLE (var);
   return var;
}

#define PTR_DEMANGLE2(var) \
   var = (typeof(var)) ptr_demangle ((uintptr_t) var)

typedef int (*callback) (void);

void
using_original_macro_demangler (callback cb)
{
   PTR_DEMANGLE (cb);
   cb();
}

void
using_new_inline_demangler (callback cb)
{
   PTR_DEMANGLE2 (cb);
   cb();
}

Re: RFC: GCC plugin to find encrypted function pointer calls in glibc

[Jeff Law]

On 04/29/2016 07:21 PM, Carlos O'Donell wrote:
> On 04/29/2016 06:23 AM, Aldy Hernandez wrote:
>> Since I would prefer not to assume the output of all inline asm's are
>> a demangling operation, I would like to get feedback from the
>> community on what would be preferred.
>
> Awesome looking work!
Agreed.  So one of the higher level questions is where should this (and 
follow-up) plugins live?  This one seems to be fairly glibc specific 
right now(*), so ISTM it ought to live in glibc in the immediate term.

The downside is if we muck things up in GCC-land and break the plugin we 
won't immediately know.  But if the plugin lives on the GCC side it's a 
lot less likely to get used regularly.

And assume that there'll be additional plugins over time where the 
plugin has some domain specific knowledge of constraint/invariant y'all 
want to enforce.  So building a little bit of infrastructure to utilize 
those plugins seems wise.

>
>> My preferred approach is to add an attribute to an inline function that would wrap the asm:
>>
>>     __attribute__((decrypt)) static inline funcp demangler (funcp f)
>>     {
>>         asm("blah");
>>     }
>>
>> This is straightforward, clean, and follows language semantics (not
>> to mention that I already have it implemented into my plugin :)), but
>> Florian made funny faces when I showed it to him, so here I am :).
>
> You can build glibc with gcc 4.7 or newer.
>
> (1) static inline wrapper with function attribute:
>
> For a representative set of architectures, say x86_64, i686, ppc64,
> s390x, aarch64, and arm, does a 4.7 or newer compiler do as good a
> job with a static inline function as it does with the macro that
> evaluates to a bare assembly?
It should, and if it doesn't for a simple case like this, I'd certainly 
treat it like a big.

>
> (2) Asm attribute:
>
> We have function, variable, type, label, and enumerator attributes,
> why not support them on asm statements to mark them as decrypt or
> encrypt functions? This would lend itself to a more natural conversion
> of inline assembly that users may want to write and embed into their
> programs.
>
> It is entirely conceivable that (1) does a good-enough job, and we
> add (2) at the same time to the latest gcc and backport. When we raise
> the supported build compiler version we can switch to (2) if needed.
GCC doesn't have any way to annotate expressions/statements in this way. 
  We can annotate types, functions and variables.

jeff

(*) We have discussed recommending other projects use function pointer 
encryption and if they do, then this plugin would be useful to those 
projects.  But that's future work ;-)

Re: RFC: GCC plugin to find encrypted function pointer calls in glibc

[Jeff Law]

On 04/30/2016 09:57 AM, Aldy Hernandez wrote:

>
> Errr, and by the way, is it a requirement that this plugin work with
> 4.7?  Because, we only need this for static analysis.  Is building glibc
> twice a no go (once with the plugin for static analysis, and once for
> building the actual production code)?  I thought the purpose of the
> plugin was just to analyze what's already there, but I do see the
> benefit of using the same compiler for static analysis _and_ for
> building the library.
>
> If the same compiler is a requirement, the plugin requires some
> rewriting, because unfortunately, the plugin infrastructure is a moving
> API target.  I doubt a plugin for GCC 6 will even compile the boiler
> plate nonsense we need for another GCC version (and vice versa).
Doesn't the build system know the version of GCC being used to compile 
glibc?  If so, could it conditionally enable/disable the plugins?

jeff

Re: RFC: GCC plugin to find encrypted function pointer calls in glibc

[Florian Weimer]

On 05/02/2016 05:26 PM, Jeff Law wrote:

> Doesn't the build system know the version of GCC being used to compile
> glibc?  If so, could it conditionally enable/disable the plugins?

I would suggest to make this a configure option at first, resulting in a 
build failure if the plugins are not buildable.  Auto-detection of a 
compatible GCC version can be tricky.  What we don't want is that the 
auto-detection determines the plugin to be incompatible, and then 
silently disables it.

Florian

Re: RFC: GCC plugin to find encrypted function pointer calls in glibc

[Roland McGrath]

The only reason we have asm in those macros is because it's optimal.
The best thing would be if we could express the bit swizzling in C in
a fashion that (new enough) GCC could be relied upon to implement with
the same optimal instruction sequence.

Using an inline with __attribute__ ((always_inline, aldys_new_magic))
should be acceptable in libc-internal sources.  But it would be better
if you have us ways to do it with less magic rather than more.

Re: RFC: GCC plugin to find encrypted function pointer calls in glibc

[Aldy Hernandez]

On 05/02/2016 06:36 PM, Roland McGrath wrote:
> The only reason we have asm in those macros is because it's optimal.
> The best thing would be if we could express the bit swizzling in C in
> a fashion that (new enough) GCC could be relied upon to implement with
> the same optimal instruction sequence.

If y'all can do it with C, be it with builtins or whatever, if there's a 
common pattern, I can probably get the plugin to notice it.  Right now, 
the only thing it notices is the result of an xor, bitwise or, or 
bitwise and (plus the proposed attribute).

> Using an inline with __attribute__ ((always_inline, aldys_new_magic))
> should be acceptable in libc-internal sources.  But it would be better
> if you have us ways to do it with less magic rather than more.

How can I say no to "aldys_new_magic attribute"?  I love it! :).

If anyone has any other suggestions, I'm all ears.  As I mentioned 
before, the only thing else I can think of is:

	asm("your stuff #some_magic_attribute#")

Assuming there's a common way to make comments across the assemblers you 
support.

Aldy

Re: RFC: GCC plugin to find encrypted function pointer calls in glibc

[Carlos O'Donell]

On 04/30/2016 11:57 AM, Aldy Hernandez wrote:
> I take it option 3 is too disgusting, provided #1 is a no go?  That
> is, adding a carefully crafted comment at the end of an inline asm
> string?

Option 3 is a non-starter.

I think a static inline wrapper is going to be just fine if the compiler
does a good job of the generated code.

> Errr, and by the way, is it a requirement that this plugin work with
> 4.7?  Because, we only need this for static analysis.  Is building
> glibc twice a no go (once with the plugin for static analysis, and
> once for building the actual production code)?  I thought the purpose
> of the plugin was just to analyze what's already there, but I do see
> the benefit of using the same compiler for static analysis _and_ for
> building the library.

You misunderstood. Let me clarify. If you add new attributes to the source
then the compiler that compiles glibc must know about those new attributes
or we must conditionally enable/disable them. If we conditionally enable
and disable them, then this will seriously impact the output of the plugin
in ways which aren't immediately obvious. Consider building with an older
gcc, and now the plugin (using a newer gcc) starts raising lots of errors.
The gcc used to build glibc should not have a material impact on the results
of the plugin.

> If the same compiler is a requirement, the plugin requires some
> rewriting, because unfortunately, the plugin infrastructure is a
> moving API target.  I doubt a plugin for GCC 6 will even compile the
> boiler plate nonsense we need for another GCC version (and vice
> versa).

That's fine.

What I would like to see is:

(a) In-tree plugins.
(b) A configure option to enable the plugins (default disabled for now).
    * Enables source markers to use the appropriate attributes.
    * Builds the plugins.
(c) Make it a hard error if a compiler recent enough to build them is
    not present.

This way the individual developers can use CI to do a glibc build with
this option enabled and look for errors.

-- 
Cheers,
Carlos.

Re: RFC: GCC plugin to find encrypted function pointer calls in glibc

[Aldy Hernandez]

On 05/04/2016 09:59 AM, Carlos O'Donell wrote:
> On 04/30/2016 11:57 AM, Aldy Hernandez wrote:
>> I take it option 3 is too disgusting, provided #1 is a no go?  That
>> is, adding a carefully crafted comment at the end of an inline asm
>> string?
>
> Option 3 is a non-starter.
>
> I think a static inline wrapper is going to be just fine if the compiler
> does a good job of the generated code.

It does, and if it doesn't, it's a compiler bug that must be fixed.

>
>> Errr, and by the way, is it a requirement that this plugin work with
>> 4.7?  Because, we only need this for static analysis.  Is building
>> glibc twice a no go (once with the plugin for static analysis, and
>> once for building the actual production code)?  I thought the purpose
>> of the plugin was just to analyze what's already there, but I do see
>> the benefit of using the same compiler for static analysis _and_ for
>> building the library.
>
> You misunderstood. Let me clarify. If you add new attributes to the source
> then the compiler that compiles glibc must know about those new attributes
> or we must conditionally enable/disable them. If we conditionally enable
> and disable them, then this will seriously impact the output of the plugin
> in ways which aren't immediately obvious. Consider building with an older
> gcc, and now the plugin (using a newer gcc) starts raising lots of errors.
> The gcc used to build glibc should not have a material impact on the results
> of the plugin.

You can leave the attributes enabled unconditionally.  GCC ignores 
unknown attributes, so the old compiler compiling the attributed source 
will behave as usual.

However, you will get an "unknown attribute ignored" warning with -Wall, 
but you can easily silence that particular warning just for the wrapper:

#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wattributes"
static inline void
__attribute__((SOME_POSSIBLY_UNKNOWN_ATTRIBUTE))
wrapper_function (void)
{
}
#pragma GCC diagnostic pop

>
>> If the same compiler is a requirement, the plugin requires some
>> rewriting, because unfortunately, the plugin infrastructure is a
>> moving API target.  I doubt a plugin for GCC 6 will even compile the
>> boiler plate nonsense we need for another GCC version (and vice
>> versa).
>
> That's fine.
>
> What I would like to see is:
>
> (a) In-tree plugins.
> (b) A configure option to enable the plugins (default disabled for now).
>      * Enables source markers to use the appropriate attributes.
>      * Builds the plugins.
> (c) Make it a hard error if a compiler recent enough to build them is
>      not present.
>
> This way the individual developers can use CI to do a glibc build with
> this option enabled and look for errors.

Sounds good.  I'll leave Florian and the rest of you glibc gurus to 
those details :).

Aldy

Re: RFC: GCC plugin to find encrypted function pointer calls in glibc

[Mikhail Maltsev]

On 05/05/2016 01:10 PM, Aldy Hernandez wrote:
> You can leave the attributes enabled unconditionally.  GCC ignores unknown
> attributes, so the old compiler compiling the attributed source will behave as
> usual.
> 
> However, you will get an "unknown attribute ignored" warning with -Wall, but you
> can easily silence that particular warning just for the wrapper:
> 
> #pragma GCC diagnostic push
> #pragma GCC diagnostic ignored "-Wattributes"
> static inline void
> __attribute__((SOME_POSSIBLY_UNKNOWN_ATTRIBUTE))
> wrapper_function (void)
> {
> }
> #pragma GCC diagnostic pop
>
Another option is:

1. Define attribute as a macro:
#ifdef __PLUGIN_LOADED__
#  define PLUGIN_ATTRIBUTE __attribute__((SOME_ATTRIBUTE))
#else
#  define PLUGIN_ATTRIBUTE /* nothing */
#endif

...

static inline void
PLUGIN_ATTRIBUTE
wrapper_function (void)
{
}

2. Define __PLUGIN_LOADED__ from PLUGIN_START_UNIT event:

static void
start_unit(void* /*event_data*/, void* /*data*/)
{
  gcc_assert(parse_in);
  cpp_define(parse_in, "__PLUGIN_LOADED__=1");
}

int
plugin_init(plugin_name_args* plugin_info, plugin_gcc_version* version)
{
  ...
  register_callback(plugin_name, PLUGIN_START_UNIT, start_unit, nullptr);
  ...
}

-- 
Regards,
    Mikhail Maltsev

Re: RFC: GCC plugin to find encrypted function pointer calls in glibc

[Aldy Hernandez]

Sorry, I thought I had already responded to this.

On 05/05/2016 08:47 AM, Mikhail Maltsev wrote:
> On 05/05/2016 01:10 PM, Aldy Hernandez wrote:
>> You can leave the attributes enabled unconditionally.  GCC ignores unknown
>> attributes, so the old compiler compiling the attributed source will behave as
>> usual.
>>
>> However, you will get an "unknown attribute ignored" warning with -Wall, but you
>> can easily silence that particular warning just for the wrapper:
>>
>> #pragma GCC diagnostic push
>> #pragma GCC diagnostic ignored "-Wattributes"
>> static inline void
>> __attribute__((SOME_POSSIBLY_UNKNOWN_ATTRIBUTE))
>> wrapper_function (void)
>> {
>> }
>> #pragma GCC diagnostic pop
>>
> Another option is:
>
> 1. Define attribute as a macro:
> #ifdef __PLUGIN_LOADED__
> #  define PLUGIN_ATTRIBUTE __attribute__((SOME_ATTRIBUTE))
> #else
> #  define PLUGIN_ATTRIBUTE /* nothing */
> #endif
>
> ...
>
> static inline void
> PLUGIN_ATTRIBUTE
> wrapper_function (void)
> {
> }
>
> 2. Define __PLUGIN_LOADED__ from PLUGIN_START_UNIT event:
>
> static void
> start_unit(void* /*event_data*/, void* /*data*/)
> {
>    gcc_assert(parse_in);
>    cpp_define(parse_in, "__PLUGIN_LOADED__=1");
> }
>
> int
> plugin_init(plugin_name_args* plugin_info, plugin_gcc_version* version)
> {
>    ...
>    register_callback(plugin_name, PLUGIN_START_UNIT, start_unit, nullptr);
>    ...
> }
>

Sounds good.  I can certainly provide a pre-processor macro if it works 
for y'all.

Also, what attribute name would folks prefer?

1. function_pointer_demangle{r}
2. pointer_demangle{r}
3. pointer_decrypt

??

I have no preference, but if no one has strong opinions, I'm arbitrarily 
choosing "pointer_demangler".

Aldy