[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: Invalid memory reads in first_non_fsm_start_char_cstring (utf_validate.c)

From: Daniel Shahaf <d.s_at_tarsus.local2>
Date: Mon Jan 4 04:30:46 2016

Branko Čibej wrote on Sun, Jan 03, 2016 at 18:12:47 +0100:
> On 03.01.2016 15:46, Hanno Böck wrote:
> > On Sat, 26 Dec 2015 12:08:12 +0100
> > Branko Čibej <brane_at_apache.org> wrote:
> >> In this case the memory is both valid (i.e., known to be
> >> allocated within the process) and properly aligned. The fact that it
> >> may not have been explicitly initialized does not affect the
> >> correctness of the code; there's no undefined behaviour being invoked
> >> here. The code relies on the fact that the size of allocated buffers
> >> is a multiple of the machine word size, which happens to be true for
> >> the APR pools we use;
> > What you're arguing here is that you're expecting certain architecture
> > and compiler specifics. But gcc may decide at any time to break your
> > assumptions.
>
> GCC (or any other compiler) may do a lot of things, but it's not allowed
> to change the way APR pool allocation works. We're not using malloc();
> we're using apr_palloc() & co.
>
> Furthermore, the code is written so that any uninitialized value it
> encounters does not affect the correctness of the result. As far as I
> can see. (And our test suite would be failing randomly otherwise.)
>

Yes, that's precisely the issue. The code assumes that the
uninitialized memory will have a value. What Hanno and Árpád are
arguing is that reading uninitialized memory is undefined behaviour and
hence the compiler is free to transform the source code to machine code
that does not correspond to any value of that uninitialized memory.

> As I said, you can always define a symbol at compile time to avoid using
> that code block.
>

I think code that makes use of undefined behaviour should be off by default.

I am not sure whether SVN_UTF_NO_UNINITIALISED_ACCESS triggers undefined
behaviour, but if it does, I think that code block should be disabled in
the default build. (I.e., the default of SVN_UTF_NO_UNINITIALISED_ACCESS
changed from 'undefined' to 'defined'.)

Cheers,

Daniel

> -- Brane
>
>
Received on 2016-01-04 04:30:46 CET

This is an archived mail posted to the Subversion Dev mailing list.