Am Montag, dem 04.12.2023 um 13:27 -0500 schrieb Siddhesh Poyarekar:
> [Branching this into a separate conversation to avoid derailing the 
> patch, which isn't directly related]
> 
> On 2023-12-04 12:21, Martin Uecker wrote:
> > I do not really agree with that.  Nested functions can substantially
> > improve code quality and in C can avoid type unsafe use of
> > void* pointers in callbacks. The code is often much better with
> > nested functions than without.  Nested functions and lambdas
> > (i.e. anonymous nested functions) are used in many languages
> > because they make code better and GNU's nested function are no
> > exception.
> > 
> > So I disagree with the idea that discouraging nested functions leads
> > to better code - I think the exact opposite is true.
> 
> I would argue that GNU's nested functions *are* an exception because 
> they're like feathers stuck on a pig to try and make it fly; I think a 
> significant specification effort is required to actually make it a 
> cleanly usable feature.  It *may* be possible to implement patterns that 
> use C nested functions well enough *and* result in readable code, but 
> IMO it is easier to write clunky and unmaintainable code with it.

I use them in my code a lot and I think they improve
code quality.  For example:

int foo_find(int N, struct foo in_array[N], const char* *key)
{
  bool cond(struct foo* x)
  {
    return 0 == strcmp(x->name, key); 
  }
  return find(N, in_array, cond);
}

is a lot cleaner and safer than what you need to write
without nested functions:

struct foo_find {
  const char* name;
}; 

int foo_cond(void *vdata, struct foo* a)
{
  struct foo *key = data;
  return 0 == strcmp(x->name, key->name);  
}

void foo_sort(int N, struct foo in_array[N], const char* key)
{
  struct foo_find data = { key };
  sort(N, in_array, foo_cond, &data);
}

and this is a toy example, the improvement gets more 
substantial with more complicated logic.

> 
> I empathize with Jakub's stated use case though of keeping the C 
> frontend support for testing purposes, but that could easily be done 
> behind a flag, or by putting nested C func deprecation behind a flag.

I am relatively sure C will get some form of nested functions.
Maybe as anonymous nested functions, i.e. lambdas, but I do
not see a fundamental difference here (I personally like naming
things for clarity, so i prefer named nested functions)

> > I am generally wary of mitigations that may make exploitation of
> > buffer overflows a bit harder  while increasing the likelihood
> > of buffer overflows by reducing type safety and/or code quality.
> > 
> > But I would agree that trampolines are generally problematic. A
> > better strategy would be wide function pointer type (as in Apple'
> > Blocks extension). Alternatively, an explicit way to obtain the
> > static chain for a nested function which could be used with
> > __builtin_call_with_static_chain  could also work.
> > 
> > But in any case, I think it diminishes the value of -fhardening
> > it if requires source code changes, because then it is not as easy
> > to simply turn it on in larger projects / distributitions.
> 
> I suppose you mean source code changes even in correct code just to 
> comply with the flag?  

Yes

> I don't disagree for cases like -Warray-bounds, 
> but for warnings/errors that are more deterministic in nature (like 
> -Werror=trampolines), they're going to point at actual problems and 
> larger projects and distributions will usually prefer to at least track 
> them, if not actually fix them.  For Fedora we tend to provide macro 
> overrides for packages that need to explicitly disable a security 
> related flag.

In projects such as mine, this will lead to a lot of code
transformations as indicated above, i.e. much worse code. 

One could get away with it, since nested functions are rarely
used, but I think this is bad, because a lot of code would
improve if it used them.

Martin

> 
> Thanks,
> Sid

Reply via email to