[FFmpeg-cvslog] r29509 - trunk/libswscale/swscale.c

[Ramiro Polla]

On Fri, Aug 14, 2009 at 8:18 AM, Reimar
D?ffinger<Reimar.Doeffinger at gmx.de> wrote:
> On Fri, Aug 14, 2009 at 10:34:07AM +0100, M?ns Rullg?rd wrote:
>> Reimar D?ffinger <Reimar.Doeffinger at gmx.de> writes:
>>
>> > On Fri, Aug 14, 2009 at 09:40:30AM +0100, M?ns Rullg?rd wrote:
>> >> Reimar D?ffinger <Reimar.Doeffinger at gmx.de> writes:
>> >>
>> >> > On Fri, Aug 14, 2009 at 02:01:12AM +0100, M?ns Rullg?rd wrote:
>> >> >> Ramiro Polla <ramiro.polla at gmail.com> writes:
>> >> >>
>> >> >> > On Thu, Aug 13, 2009 at 4:24 PM, Reimar
>> >> >> > D?ffinger<Reimar.Doeffinger at gmx.de> wrote:
>> >> >> >> On Thu, Aug 13, 2009 at 08:29:54PM +0200, ramiro wrote:
>> >> >> >>> +#ifdef MAP_ANONYMOUS
>> >> >> >>> + ? ? ? ? ? ?mprotect(c->lumMmx2FilterCode, MAX_MMX2_FILTER_CODE_SIZE, PROT_EXEC | PROT_READ);
>> >> >> >>> + ? ? ? ? ? ?mprotect(c->chrMmx2FilterCode, MAX_MMX2_FILTER_CODE_SIZE, PROT_EXEC | PROT_READ);
>> >> >> >>
>> >> >> >> Both for mmap and mprotect: Why PROT_READ though? I don't think
>> >> >> >> EXEC without read is possible on most architectures, but still we
>> >> >> >> do not need read access in any case I think...
>> >> >> >
>> >> >> > Mans, or anyone else that knows an extensive amount of platforms, can
>> >> >> > you comment on this?
>> >> >>
>> >> >> On ARM read permission is required to execute. ?I see no harm in
>> >> >> always requesting both.
>> >> >
>> >> > mmap/mprotect can and will automatically "upgrade" the permissions.
>> >>
>> >> On some systems perhaps, but these are arguably not strictly
>> >> conforming to spec. ?This is from the spec page for mmap():
>> >>
>> >> ? If an implementation cannot support the combination of access types
>> >> ? specified by prot, the call to mmap() shall fail.
>> >>
>> >> It says the same thing about mprotect(). ?It is of course unreasonable
>> >> to expect the caller to know the precise combinations possible on
>> >> every bit of hardware, so adjusting to something it can do is a good
>> >> idea. ?It's probably not compliant to the letter though.
>> >
>> > Uh, you forgot to quote the following sentence, which clarifies what
>> > the thing you quoted meant:
>> >> An implementation may permit accesses other than those specified by
>> >> prot;
>> >
>> > The case where it "shall fail" is e.g. a system where there is no memory
>> > that would allow both executing and reading the data in principle,
>> > then PROT_READ | PROT_EXEC mmap shall fail (instead of not giving one of
>> > the requested permissions).
>>
>> That is probably the intent, but that's not what it says.
>
> It is what it says. What you read into it is "If an implementation
> cannot support the _exact_ combination" - which I admit is _one_
> reasonable way to read it, but not in the context...
> E.g. by supporting READ+WRITE+EXEC at once an architecture also
> supports READ or READ+WRITE or READ+EXEC etc., it just does not
> deny _all_ other types of access.
> Whereas an architecture where e.g. all code is in ROM indeed really
> and in no way can support anything with WRITE+EXEC.

Hmm, I just remembered this code is strictly x86+mmx2. It has worked
up to now. We could remove READ and wait for a possible regression, or
leave it the way we know it works. I prefer just leaving it as is.