Re: RFC: tunables failure indications...

[Siddhesh Poyarekar <siddhesh at gotplt dot org>]

On 06/12/19 4:14 am, DJ Delorie wrote:
> Re: https://sourceware.org/bugzilla/show_bug.cgi?id=25035
> "sbrk() failure handled poorly in tunables_strdup"

As I understand it, there seem to be a few different problems in that bug:

1. The env gets updated even when strdup returns NULL, which is a bug
2. Tunables may get ignored on internal errors
3. Tunables may may get on invalid input
4. brk failed

> The current failure mode for tunables is: if we can't allocate memory,
> we delete the tunables env var and pretend it didn't exist, without
> telling the user anything happened.

That was a deliberate design decision back then because tunables were
considered non-critical, i.e. they're not guaranteed to work in all
situations and as such, shouldn't hamper core functionality just because
it couldn't work.  So problem (2) and (3) are by design.

> I think we should do better, but I don't know what.
> 
> IMHO tunables are "hints" but there should be *some* feedback when a
> tunable can't be honored because of internal failures (vs tunables
> that don't apply to the current arch, etc).
> 
> In the BZ noted above, where *sbrk* fails, I'm inclined to just
> SIGSEGV because... well... *sbrk* just failed at the start of a
> program.  Something has gone horribly wrong, even if the program
> continues to run normally after that (it happens in our own testsuite
> sometimes).

That's a fair point.  I would agree with a patch that fails
catastrophically on a failed brk or similar internal errors as opposed
to invalid values from users.  That is, we make (2) fail with an error exit.

> The helper functions in malloc return error codes for invalid values,
> which the tunables harness completely ignores.  Not even a printf.

This replicates what we used to do earlier when setting the
MALLOC_MMAP_MAX_, etc environment variables and it follows the same
principle that it shouldn't hamper core functionality.  Maybe there's a
valid use case to have debug messages (when built with a flag set for
example) that get explicitly enabled to dump warnings on invalid tunable
values.

As for (4), that's super-odd even with randomization.  Is it possible
that somehow brk with ASLR enabled fails when the new brk end is not
page aligned?

Siddhesh