[Devel] [RFC] proposed patch for some segfaults encountered

[Devel] [RFC] proposed patch for some segfaults encountered

[Al Stone]

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

It's not clear to me that this is the correct way to handle a segfault that
was reported to me via Fedora.

Using the ssdt9.dat file attached, a simple "iasl -d ssdt9.dat" will segfault
around calls to AcpiDmPromoteTarget -- and it turns out that the Target pointer
being passed into that function was sometimes null but not being checked in the
calling function, AcpiDmCheckForSymbolicOpcode.  So, this patch adds in some
checks for null pointers and can now get through this particular problem.

However, I did not study AcpiDmCheckForSymbolicOpcode long enough to determine
if the Target pointer in that function should or should not ever be null.  So,
this patch fixes this issue, but it may not be the proper long term solution.

-- 
ciao,
al
-----------------------------------
Al Stone
Software Engineer
Red Hat, Inc.
ahs3(a)redhat.com
-----------------------------------

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: segfault.patch --]
[-- Type: text/x-patch, Size: 1207 bytes --]

diff -Naur acpica-unix2-20150515.orig/source/components/disassembler/dmcstyle.c acpica-unix2-20150515/source/components/disassembler/dmcstyle.c
--- acpica-unix2-20150515.orig/source/components/disassembler/dmcstyle.c	2015-06-01 14:32:51.769218887 -0600
+++ acpica-unix2-20150515/source/components/disassembler/dmcstyle.c	2015-06-01 14:37:38.973898839 -0600
@@ -407,7 +407,7 @@
         {
             AcpiDmPromoteTarget (Op, Target);
 
-            if (!Target->Common.OperatorSymbol)
+            if (Target && !Target->Common.OperatorSymbol)
             {
                 Target->Common.OperatorSymbol = " = ";
             }
@@ -425,7 +425,7 @@
         Target = Child1->Common.Next;
         AcpiDmPromoteTarget (Op, Target);
 
-        if (!Target->Common.OperatorSymbol)
+        if (Target && !Target->Common.OperatorSymbol)
         {
             Target->Common.OperatorSymbol = " = ";
         }
@@ -446,7 +446,10 @@
             /* Valid target, not a placeholder */
 
             AcpiDmPromoteTarget (Op, Target);
-            Target->Common.OperatorSymbol = " = ~";
+	    if (Target)
+	    {
+            	Target->Common.OperatorSymbol = " = ~";
+	    }
         }
         else
         {

Re: [Devel] [RFC] proposed patch for some segfaults encountered

[Moore, Robert]

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

I think we have taken at least one look at this one. We added checks for null pointers, but then the compiler failed our test suite. So, we feel that the problem is a bit more complex. However, it is on our list of things to do.


> -----Original Message-----
> From: Devel [mailto:devel-bounces(a)acpica.org] On Behalf Of Al Stone
> Sent: Monday, June 01, 2015 1:59 PM
> To: devel(a)acpica.org
> Subject: [Devel] [RFC] proposed patch for some segfaults encountered
> 
> It's not clear to me that this is the correct way to handle a segfault
> that was reported to me via Fedora.
> 
> Using the ssdt9.dat file attached, a simple "iasl -d ssdt9.dat" will
> segfault around calls to AcpiDmPromoteTarget -- and it turns out that the
> Target pointer being passed into that function was sometimes null but not
> being checked in the calling function, AcpiDmCheckForSymbolicOpcode.  So,
> this patch adds in some checks for null pointers and can now get through
> this particular problem.
> 
> However, I did not study AcpiDmCheckForSymbolicOpcode long enough to
> determine if the Target pointer in that function should or should not ever
> be null.  So, this patch fixes this issue, but it may not be the proper
> long term solution.
> 
> --
> ciao,
> al
> -----------------------------------
> Al Stone
> Software Engineer
> Red Hat, Inc.
> ahs3(a)redhat.com
> -----------------------------------

Re: [Devel] [RFC] proposed patch for some segfaults encountered

[Al Stone]

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

Interesting.  I'll double check the results from ASLTS but I don't recall
seeing any changes.

On 06/01/2015 03:58 PM, Moore, Robert wrote:
> I think we have taken at least one look at this one. We added checks for null pointers, but then the compiler failed our test suite. So, we feel that the problem is a bit more complex. However, it is on our list of things to do.
> 
> 
>> -----Original Message-----
>> From: Devel [mailto:devel-bounces(a)acpica.org] On Behalf Of Al Stone
>> Sent: Monday, June 01, 2015 1:59 PM
>> To: devel(a)acpica.org
>> Subject: [Devel] [RFC] proposed patch for some segfaults encountered
>>
>> It's not clear to me that this is the correct way to handle a segfault
>> that was reported to me via Fedora.
>>
>> Using the ssdt9.dat file attached, a simple "iasl -d ssdt9.dat" will
>> segfault around calls to AcpiDmPromoteTarget -- and it turns out that the
>> Target pointer being passed into that function was sometimes null but not
>> being checked in the calling function, AcpiDmCheckForSymbolicOpcode.  So,
>> this patch adds in some checks for null pointers and can now get through
>> this particular problem.
>>
>> However, I did not study AcpiDmCheckForSymbolicOpcode long enough to
>> determine if the Target pointer in that function should or should not ever
>> be null.  So, this patch fixes this issue, but it may not be the proper
>> long term solution.
>>
>> --
>> ciao,
>> al
>> -----------------------------------
>> Al Stone
>> Software Engineer
>> Red Hat, Inc.
>> ahs3(a)redhat.com
>> -----------------------------------


-- 
ciao,
al
-----------------------------------
Al Stone
Software Engineer
Red Hat, Inc.
ahs3(a)redhat.com
-----------------------------------

Re: [Devel] [RFC] proposed patch for some segfaults encountered

[Moore, Robert]

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

Please send the ssdt9.dat file, it wasn't attached.
Bob


> -----Original Message-----
> From: Devel [mailto:devel-bounces(a)acpica.org] On Behalf Of Al Stone
> Sent: Monday, June 01, 2015 1:59 PM
> To: devel(a)acpica.org
> Subject: [Devel] [RFC] proposed patch for some segfaults encountered
> 
> It's not clear to me that this is the correct way to handle a segfault
> that was reported to me via Fedora.
> 
> Using the ssdt9.dat file attached, a simple "iasl -d ssdt9.dat" will
> segfault around calls to AcpiDmPromoteTarget -- and it turns out that the
> Target pointer being passed into that function was sometimes null but not
> being checked in the calling function, AcpiDmCheckForSymbolicOpcode.  So,
> this patch adds in some checks for null pointers and can now get through
> this particular problem.
> 
> However, I did not study AcpiDmCheckForSymbolicOpcode long enough to
> determine if the Target pointer in that function should or should not ever
> be null.  So, this patch fixes this issue, but it may not be the proper
> long term solution.
> 
> --
> ciao,
> al
> -----------------------------------
> Al Stone
> Software Engineer
> Red Hat, Inc.
> ahs3(a)redhat.com
> -----------------------------------

Re: [Devel] [RFC] proposed patch for some segfaults encountered

[Al Stone]

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

On 06/02/2015 10:26 AM, Moore, Robert wrote:
> Please send the ssdt9.dat file, it wasn't attached.
> Bob
> 

Ooops.  Sorry about that.

-- 
ciao,
al
-----------------------------------
Al Stone
Software Engineer
Red Hat, Inc.
ahs3(a)redhat.com
-----------------------------------

[-- Attachment #2: ssdt9.dat --]
[-- Type: application/octet-stream, Size: 3102 bytes --]

Re: [Devel] [RFC] proposed patch for some segfaults encountered

[Moore, Robert]

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

Ok, I've been able to reproduce the problem. I'll let you know how things progress.
Bob


> -----Original Message-----
> From: Al Stone [mailto:ahs3(a)redhat.com]
> Sent: Tuesday, June 02, 2015 9:48 AM
> To: Moore, Robert; devel(a)acpica.org
> Subject: Re: [Devel] [RFC] proposed patch for some segfaults encountered
> 
> On 06/02/2015 10:26 AM, Moore, Robert wrote:
> > Please send the ssdt9.dat file, it wasn't attached.
> > Bob
> >
> 
> Ooops.  Sorry about that.
> 
> --
> ciao,
> al
> -----------------------------------
> Al Stone
> Software Engineer
> Red Hat, Inc.
> ahs3(a)redhat.com
> -----------------------------------

Re: [Devel] [RFC] proposed patch for some segfaults encountered

[Moore, Robert]

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

I'll need to disassemble with the DSDT and all SSDTs.

Do you have an acpidump for the machine? Otherwise, the binary files will be ok.

There are a lot of externals to other tables, and this may be a part of the problem.


> -----Original Message-----
> From: Moore, Robert
> Sent: Tuesday, June 02, 2015 11:55 AM
> To: 'Al Stone'; devel(a)acpica.org
> Subject: RE: [Devel] [RFC] proposed patch for some segfaults encountered
> 
> Ok, I've been able to reproduce the problem. I'll let you know how things
> progress.
> Bob
> 
> 
> > -----Original Message-----
> > From: Al Stone [mailto:ahs3(a)redhat.com]
> > Sent: Tuesday, June 02, 2015 9:48 AM
> > To: Moore, Robert; devel(a)acpica.org
> > Subject: Re: [Devel] [RFC] proposed patch for some segfaults encountered
> >
> > On 06/02/2015 10:26 AM, Moore, Robert wrote:
> > > Please send the ssdt9.dat file, it wasn't attached.
> > > Bob
> > >
> >
> > Ooops.  Sorry about that.
> >
> > --
> > ciao,
> > al
> > -----------------------------------
> > Al Stone
> > Software Engineer
> > Red Hat, Inc.
> > ahs3(a)redhat.com
> > -----------------------------------

Re: [Devel] [RFC] proposed patch for some segfaults encountered

[Moore, Robert]

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

It looks to me that the disassembler has to start guessing at the number of arguments for the various external control methods, and it begins to get seriously confused.

Just FYI, here is a full description of the problem:

4.4.1	Disassembly of Control Method Invocations

Summary of the external control method problem: 
When the -e option is used with disassembly, the various SSDTs are simply loaded into a global namespace for the disassembler to use in order to resolve control method references (invocations). 
The disassembler tracks any such references, and will emit an External() statement for these types of methods, with the proper number of arguments . 
Without the SSDTs, the AML does not contain enough information to properly disassemble the control method invocation -- because the disassembler does not know how many arguments to parse. 
An example: Assume we have two control methods. ABCD has one argument, and EFGH has zero arguments. Further, we have two additional control methods that invoke ABCD and EFGH, named T1 and T2: 

    Method (ABCD, 1)
    {
    }
    Method (EFGH, 0)
    {
    }
    Method (T1)
    {
        ABCD (Add (2, 7, Local0))
    }
    Method (T2)
    {
        EFGH ()
        Add (2, 7, Local0)
    }

Here is the AML code that is generated for T1 and T2:

     185:      Method (T1)

0000034C:  14 10 54 31 5F 5F 00 ...    "..T1__."

     186:      {
     187:          ABCD (Add (2, 7, Local0))

00000353:  41 42 43 44 ............    "ABCD"
00000357:  72 0A 02 0A 07 60 ......    "r....`"

     188:      }

     190:      Method (T2)

0000035D:  14 10 54 32 5F 5F 00 ...    "..T2__."

     191:      {
     192:          EFGH ()

00000364:  45 46 47 48 ............    "EFGH"

     193:          Add (2, 7, Local0)

00000368:  72 0A 02 0A 07 60 ......    "r....`"
     194:      }

Note that the AML code for T1 and T2 is essentially identical. When disassembling this code, the methods ABCD and EFGH must be known to the disassembler, otherwise it does not know how to handle the method invocations. 
In other words, if ABCD and EFGH are actually external control methods appearing in an SSDT, the disassembler does not know what to do unless the owning SSDT has been loaded via the -e option. 

4.4.1.1	ACPI 6.0 Solution - External() AML Opcode
In ACPI 6.0, a new AML opcode was defined to help with this problem. The opcode is not intended for execution by AML interpreters; its only purpose is to assist the disassembler with the control method argument count problem.




> -----Original Message-----
> From: Devel [mailto:devel-bounces(a)acpica.org] On Behalf Of Moore, Robert
> Sent: Tuesday, June 02, 2015 12:06 PM
> To: Al Stone; devel(a)acpica.org
> Subject: Re: [Devel] [RFC] proposed patch for some segfaults encountered
> 
> I'll need to disassemble with the DSDT and all SSDTs.
> 
> Do you have an acpidump for the machine? Otherwise, the binary files will
> be ok.
> 
> There are a lot of externals to other tables, and this may be a part of
> the problem.
> 
> 

Re: [Devel] [RFC] proposed patch for some segfaults encountered

[Al Stone]

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

On 06/02/2015 01:06 PM, Moore, Robert wrote:
> I'll need to disassemble with the DSDT and all SSDTs.
> 
> Do you have an acpidump for the machine? Otherwise, the binary files will be ok.
> 
> There are a lot of externals to other tables, and this may be a part of the problem.

Aha.  So noted from the follow-on message.

I'll see if I can get the full tables.  All that was provided to me was the
one SSDT, but I'll ask the bug reporter.

>> -----Original Message-----
>> From: Moore, Robert
>> Sent: Tuesday, June 02, 2015 11:55 AM
>> To: 'Al Stone'; devel(a)acpica.org
>> Subject: RE: [Devel] [RFC] proposed patch for some segfaults encountered
>>
>> Ok, I've been able to reproduce the problem. I'll let you know how things
>> progress.
>> Bob
>>
>>
>>> -----Original Message-----
>>> From: Al Stone [mailto:ahs3(a)redhat.com]
>>> Sent: Tuesday, June 02, 2015 9:48 AM
>>> To: Moore, Robert; devel(a)acpica.org
>>> Subject: Re: [Devel] [RFC] proposed patch for some segfaults encountered
>>>
>>> On 06/02/2015 10:26 AM, Moore, Robert wrote:
>>>> Please send the ssdt9.dat file, it wasn't attached.
>>>> Bob
>>>>
>>>
>>> Ooops.  Sorry about that.
>>>
>>> --
>>> ciao,
>>> al
>>> -----------------------------------
>>> Al Stone
>>> Software Engineer
>>> Red Hat, Inc.
>>> ahs3(a)redhat.com
>>> -----------------------------------


-- 
ciao,
al
-----------------------------------
Al Stone
Software Engineer
Red Hat, Inc.
ahs3(a)redhat.com
-----------------------------------

Re: [Devel] [RFC] proposed patch for some segfaults encountered

[Zheng, Lv]

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

Hi,

> From: Devel [mailto:devel-bounces(a)acpica.org] On Behalf Of Moore, Robert
> Sent: Wednesday, June 03, 2015 4:06 AM
> 
> It looks to me that the disassembler has to start guessing at the number of arguments for the various external control methods, and it
> begins to get seriously confused.
> 
> Just FYI, here is a full description of the problem:
> 
> 4.4.1	Disassembly of Control Method Invocations
> 
> Summary of the external control method problem:
> When the -e option is used with disassembly, the various SSDTs are simply loaded into a global namespace for the disassembler to use
> in order to resolve control method references (invocations).
> The disassembler tracks any such references, and will emit an External() statement for these types of methods, with the proper
> number of arguments .
> Without the SSDTs, the AML does not contain enough information to properly disassemble the control method invocation -- because
> the disassembler does not know how many arguments to parse.
> An example: Assume we have two control methods. ABCD has one argument, and EFGH has zero arguments. Further, we have two
> additional control methods that invoke ABCD and EFGH, named T1 and T2:
> 
>     Method (ABCD, 1)
>     {
>     }
>     Method (EFGH, 0)
>     {
>     }
>     Method (T1)
>     {
>         ABCD (Add (2, 7, Local0))
>     }
>     Method (T2)
>     {
>         EFGH ()
>         Add (2, 7, Local0)
>     }
> 
> Here is the AML code that is generated for T1 and T2:
> 
>      185:      Method (T1)
> 
> 0000034C:  14 10 54 31 5F 5F 00 ...    "..T1__."
> 
>      186:      {
>      187:          ABCD (Add (2, 7, Local0))
> 
> 00000353:  41 42 43 44 ............    "ABCD"
> 00000357:  72 0A 02 0A 07 60 ......    "r....`"
> 
>      188:      }
> 
>      190:      Method (T2)
> 
> 0000035D:  14 10 54 32 5F 5F 00 ...    "..T2__."
> 
>      191:      {
>      192:          EFGH ()
> 
> 00000364:  45 46 47 48 ............    "EFGH"
> 
>      193:          Add (2, 7, Local0)
> 
> 00000368:  72 0A 02 0A 07 60 ......    "r....`"
>      194:      }
> 
> Note that the AML code for T1 and T2 is essentially identical. When disassembling this code, the methods ABCD and EFGH must be
> known to the disassembler, otherwise it does not know how to handle the method invocations.

This is the problem in ACPI specification since DSDT 1.0:
MethodInvocation := NameString TermArgList
TermObj := NameSpaceModifierObj | NamedObj | Type1Opcode | Type2Opcode
TermList := Nothing | <TermObj TermList>
There is no number of arguments defined in the grammer.

> In other words, if ABCD and EFGH are actually external control methods appearing in an SSDT, the disassembler does not know what
> to do unless the owning SSDT has been loaded via the -e option.

Normally  I will use the following command to decompile:
# iasl -e dsdt.dat -e ssdt*.dat -d ssdt9.dat
Which loads all tables in the same order as the OSPM (may still face issue if there is "Load/LoadTable" invoked during the table loading).
If this still failed, this was either a BIOS bug or an DSDT 2.0 compatible problem.

Thanks and best regards
-Lv

> 
> 4.4.1.1	ACPI 6.0 Solution - External() AML Opcode
> In ACPI 6.0, a new AML opcode was defined to help with this problem. The opcode is not intended for execution by AML interpreters;
> its only purpose is to assist the disassembler with the control method argument count problem.
> 
> 
> 
> 
> > -----Original Message-----
> > From: Devel [mailto:devel-bounces(a)acpica.org] On Behalf Of Moore, Robert
> > Sent: Tuesday, June 02, 2015 12:06 PM
> > To: Al Stone; devel(a)acpica.org
> > Subject: Re: [Devel] [RFC] proposed patch for some segfaults encountered
> >
> > I'll need to disassemble with the DSDT and all SSDTs.
> >
> > Do you have an acpidump for the machine? Otherwise, the binary files will
> > be ok.
> >
> > There are a lot of externals to other tables, and this may be a part of
> > the problem.
> >
> >
> _______________________________________________
> Devel mailing list
> Devel(a)acpica.org
> https://lists.acpica.org/mailman/listinfo/devel

Re: [Devel] [RFC] proposed patch for some segfaults encountered

[Moore, Robert]

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

And of course, the warning message should be taken to heart:

    /*
     * iASL Warning: There were 4 external control methods found during
     * disassembly, but additional ACPI tables to resolve these externals
     * were not specified. This resulting disassembler output file may not
     * compile because the disassembler did not know how many arguments
     * to assign to these methods. To specify the tables needed to resolve
     * external control method references, the -e option can be used to
     * specify the filenames. Note: SSDTs can be dynamically loaded at
     * runtime and may or may not be available via the host OS.
     * Example iASL invocations:
     *     iasl -e ssdt1.aml ssdt2.aml ssdt3.aml -d dsdt.aml
     *     iasl -e dsdt.aml ssdt2.aml -d ssdt1.aml
     *     iasl -e ssdt*.aml -d dsdt.aml
     *
     * In addition, the -fe option can be used to specify a file containing
     * control method external declarations with the associated method
     * argument counts. Each line of the file must be of the form:
     *     External (<method pathname>, MethodObj, <argument count>)
     * Invocation:
     *     iasl -fe refs.txt -d dsdt.aml
     *
     * The following methods were unresolved and many not compile properly
     * because the disassembler had to guess at the number of arguments
     * required for each:
     */


> -----Original Message-----
> From: Zheng, Lv
> Sent: Tuesday, June 02, 2015 5:38 PM
> To: Moore, Robert; Al Stone; devel(a)acpica.org
> Subject: RE: [Devel] [RFC] proposed patch for some segfaults encountered
> 
> Hi,
> 
> > From: Devel [mailto:devel-bounces(a)acpica.org] On Behalf Of Moore,
> > Robert
> > Sent: Wednesday, June 03, 2015 4:06 AM
> >
> > It looks to me that the disassembler has to start guessing at the
> > number of arguments for the various external control methods, and it
> begins to get seriously confused.
> >
> > Just FYI, here is a full description of the problem:
> >
> > 4.4.1	Disassembly of Control Method Invocations
> >
> > Summary of the external control method problem:
> > When the -e option is used with disassembly, the various SSDTs are
> > simply loaded into a global namespace for the disassembler to use in
> order to resolve control method references (invocations).
> > The disassembler tracks any such references, and will emit an
> > External() statement for these types of methods, with the proper number
> of arguments .
> > Without the SSDTs, the AML does not contain enough information to
> > properly disassemble the control method invocation -- because the
> disassembler does not know how many arguments to parse.
> > An example: Assume we have two control methods. ABCD has one argument,
> > and EFGH has zero arguments. Further, we have two additional control
> methods that invoke ABCD and EFGH, named T1 and T2:
> >
> >     Method (ABCD, 1)
> >     {
> >     }
> >     Method (EFGH, 0)
> >     {
> >     }
> >     Method (T1)
> >     {
> >         ABCD (Add (2, 7, Local0))
> >     }
> >     Method (T2)
> >     {
> >         EFGH ()
> >         Add (2, 7, Local0)
> >     }
> >
> > Here is the AML code that is generated for T1 and T2:
> >
> >      185:      Method (T1)
> >
> > 0000034C:  14 10 54 31 5F 5F 00 ...    "..T1__."
> >
> >      186:      {
> >      187:          ABCD (Add (2, 7, Local0))
> >
> > 00000353:  41 42 43 44 ............    "ABCD"
> > 00000357:  72 0A 02 0A 07 60 ......    "r....`"
> >
> >      188:      }
> >
> >      190:      Method (T2)
> >
> > 0000035D:  14 10 54 32 5F 5F 00 ...    "..T2__."
> >
> >      191:      {
> >      192:          EFGH ()
> >
> > 00000364:  45 46 47 48 ............    "EFGH"
> >
> >      193:          Add (2, 7, Local0)
> >
> > 00000368:  72 0A 02 0A 07 60 ......    "r....`"
> >      194:      }
> >
> > Note that the AML code for T1 and T2 is essentially identical. When
> > disassembling this code, the methods ABCD and EFGH must be known to the
> disassembler, otherwise it does not know how to handle the method
> invocations.
> 
> This is the problem in ACPI specification since DSDT 1.0:
> MethodInvocation := NameString TermArgList TermObj := NameSpaceModifierObj
> | NamedObj | Type1Opcode | Type2Opcode TermList := Nothing | <TermObj
> TermList> There is no number of arguments defined in the grammer.
> 
> > In other words, if ABCD and EFGH are actually external control methods
> > appearing in an SSDT, the disassembler does not know what to do unless
> the owning SSDT has been loaded via the -e option.
> 
> Normally  I will use the following command to decompile:
> # iasl -e dsdt.dat -e ssdt*.dat -d ssdt9.dat Which loads all tables in the
> same order as the OSPM (may still face issue if there is "Load/LoadTable"
> invoked during the table loading).
> If this still failed, this was either a BIOS bug or an DSDT 2.0 compatible
> problem.
> 
> Thanks and best regards
> -Lv
> 
> >
> > 4.4.1.1	ACPI 6.0 Solution - External() AML Opcode
> > In ACPI 6.0, a new AML opcode was defined to help with this problem.
> > The opcode is not intended for execution by AML interpreters; its only
> purpose is to assist the disassembler with the control method argument
> count problem.
> >
> >
> >
> >
> > > -----Original Message-----
> > > From: Devel [mailto:devel-bounces(a)acpica.org] On Behalf Of Moore,
> > > Robert
> > > Sent: Tuesday, June 02, 2015 12:06 PM
> > > To: Al Stone; devel(a)acpica.org
> > > Subject: Re: [Devel] [RFC] proposed patch for some segfaults
> > > encountered
> > >
> > > I'll need to disassemble with the DSDT and all SSDTs.
> > >
> > > Do you have an acpidump for the machine? Otherwise, the binary files
> > > will be ok.
> > >
> > > There are a lot of externals to other tables, and this may be a part
> > > of the problem.
> > >
> > >
> > _______________________________________________
> > Devel mailing list
> > Devel(a)acpica.org
> > https://lists.acpica.org/mailman/listinfo/devel

Re: [Devel] [RFC] proposed patch for some segfaults encountered

[Al Stone]

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

Yup.  All true and duly noted.  Unfortunately, I could only pass
on what I've been given from the bug report.  And, the warning was
not showing up because of the segfault.  Now that it does, it should
help some.

The Fedora BZ, if you're interested, is here:

   https://bugzilla.redhat.com/show_bug.cgi?id=1219341


On 06/03/2015 08:57 AM, Moore, Robert wrote:
> And of course, the warning message should be taken to heart:
> 
>     /*
>      * iASL Warning: There were 4 external control methods found during
>      * disassembly, but additional ACPI tables to resolve these externals
>      * were not specified. This resulting disassembler output file may not
>      * compile because the disassembler did not know how many arguments
>      * to assign to these methods. To specify the tables needed to resolve
>      * external control method references, the -e option can be used to
>      * specify the filenames. Note: SSDTs can be dynamically loaded at
>      * runtime and may or may not be available via the host OS.
>      * Example iASL invocations:
>      *     iasl -e ssdt1.aml ssdt2.aml ssdt3.aml -d dsdt.aml
>      *     iasl -e dsdt.aml ssdt2.aml -d ssdt1.aml
>      *     iasl -e ssdt*.aml -d dsdt.aml
>      *
>      * In addition, the -fe option can be used to specify a file containing
>      * control method external declarations with the associated method
>      * argument counts. Each line of the file must be of the form:
>      *     External (<method pathname>, MethodObj, <argument count>)
>      * Invocation:
>      *     iasl -fe refs.txt -d dsdt.aml
>      *
>      * The following methods were unresolved and many not compile properly
>      * because the disassembler had to guess at the number of arguments
>      * required for each:
>      */
> 
> 
>> -----Original Message-----
>> From: Zheng, Lv
>> Sent: Tuesday, June 02, 2015 5:38 PM
>> To: Moore, Robert; Al Stone; devel(a)acpica.org
>> Subject: RE: [Devel] [RFC] proposed patch for some segfaults encountered
>>
>> Hi,
>>
>>> From: Devel [mailto:devel-bounces(a)acpica.org] On Behalf Of Moore,
>>> Robert
>>> Sent: Wednesday, June 03, 2015 4:06 AM
>>>
>>> It looks to me that the disassembler has to start guessing at the
>>> number of arguments for the various external control methods, and it
>> begins to get seriously confused.
>>>
>>> Just FYI, here is a full description of the problem:
>>>
>>> 4.4.1	Disassembly of Control Method Invocations
>>>
>>> Summary of the external control method problem:
>>> When the -e option is used with disassembly, the various SSDTs are
>>> simply loaded into a global namespace for the disassembler to use in
>> order to resolve control method references (invocations).
>>> The disassembler tracks any such references, and will emit an
>>> External() statement for these types of methods, with the proper number
>> of arguments .
>>> Without the SSDTs, the AML does not contain enough information to
>>> properly disassemble the control method invocation -- because the
>> disassembler does not know how many arguments to parse.
>>> An example: Assume we have two control methods. ABCD has one argument,
>>> and EFGH has zero arguments. Further, we have two additional control
>> methods that invoke ABCD and EFGH, named T1 and T2:
>>>
>>>     Method (ABCD, 1)
>>>     {
>>>     }
>>>     Method (EFGH, 0)
>>>     {
>>>     }
>>>     Method (T1)
>>>     {
>>>         ABCD (Add (2, 7, Local0))
>>>     }
>>>     Method (T2)
>>>     {
>>>         EFGH ()
>>>         Add (2, 7, Local0)
>>>     }
>>>
>>> Here is the AML code that is generated for T1 and T2:
>>>
>>>      185:      Method (T1)
>>>
>>> 0000034C:  14 10 54 31 5F 5F 00 ...    "..T1__."
>>>
>>>      186:      {
>>>      187:          ABCD (Add (2, 7, Local0))
>>>
>>> 00000353:  41 42 43 44 ............    "ABCD"
>>> 00000357:  72 0A 02 0A 07 60 ......    "r....`"
>>>
>>>      188:      }
>>>
>>>      190:      Method (T2)
>>>
>>> 0000035D:  14 10 54 32 5F 5F 00 ...    "..T2__."
>>>
>>>      191:      {
>>>      192:          EFGH ()
>>>
>>> 00000364:  45 46 47 48 ............    "EFGH"
>>>
>>>      193:          Add (2, 7, Local0)
>>>
>>> 00000368:  72 0A 02 0A 07 60 ......    "r....`"
>>>      194:      }
>>>
>>> Note that the AML code for T1 and T2 is essentially identical. When
>>> disassembling this code, the methods ABCD and EFGH must be known to the
>> disassembler, otherwise it does not know how to handle the method
>> invocations.
>>
>> This is the problem in ACPI specification since DSDT 1.0:
>> MethodInvocation := NameString TermArgList TermObj := NameSpaceModifierObj
>> | NamedObj | Type1Opcode | Type2Opcode TermList := Nothing | <TermObj
>> TermList> There is no number of arguments defined in the grammer.
>>
>>> In other words, if ABCD and EFGH are actually external control methods
>>> appearing in an SSDT, the disassembler does not know what to do unless
>> the owning SSDT has been loaded via the -e option.
>>
>> Normally  I will use the following command to decompile:
>> # iasl -e dsdt.dat -e ssdt*.dat -d ssdt9.dat Which loads all tables in the
>> same order as the OSPM (may still face issue if there is "Load/LoadTable"
>> invoked during the table loading).
>> If this still failed, this was either a BIOS bug or an DSDT 2.0 compatible
>> problem.
>>
>> Thanks and best regards
>> -Lv
>>
>>>
>>> 4.4.1.1	ACPI 6.0 Solution - External() AML Opcode
>>> In ACPI 6.0, a new AML opcode was defined to help with this problem.
>>> The opcode is not intended for execution by AML interpreters; its only
>> purpose is to assist the disassembler with the control method argument
>> count problem.
>>>
>>>
>>>
>>>
>>>> -----Original Message-----
>>>> From: Devel [mailto:devel-bounces(a)acpica.org] On Behalf Of Moore,
>>>> Robert
>>>> Sent: Tuesday, June 02, 2015 12:06 PM
>>>> To: Al Stone; devel(a)acpica.org
>>>> Subject: Re: [Devel] [RFC] proposed patch for some segfaults
>>>> encountered
>>>>
>>>> I'll need to disassemble with the DSDT and all SSDTs.
>>>>
>>>> Do you have an acpidump for the machine? Otherwise, the binary files
>>>> will be ok.
>>>>
>>>> There are a lot of externals to other tables, and this may be a part
>>>> of the problem.
>>>>
>>>>
>>> _______________________________________________
>>> Devel mailing list
>>> Devel(a)acpica.org
>>> https://lists.acpica.org/mailman/listinfo/devel


-- 
ciao,
al
-----------------------------------
Al Stone
Software Engineer
Red Hat, Inc.
ahs3(a)redhat.com
-----------------------------------

Re: [Devel] [RFC] proposed patch for some segfaults encountered

[Moore, Robert]

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

As far as the segfault, this issue becomes very problematic as the disassembler parse tree can become ill-formed and the results are somewhat unpredictable. However, we will continue to do what we can short of checking every single node in the parse tree for validity, which of course is a lot of work, CPU time, or both.  Obviously, we want to of course make the disassembler as bulletproof as we can.



> -----Original Message-----
> From: Al Stone [mailto:ahs3(a)redhat.com]
> Sent: Wednesday, June 03, 2015 12:54 PM
> To: Moore, Robert; Zheng, Lv; devel(a)acpica.org
> Subject: Re: [Devel] [RFC] proposed patch for some segfaults encountered
> 
> Yup.  All true and duly noted.  Unfortunately, I could only pass on what
> I've been given from the bug report.  And, the warning was not showing up
> because of the segfault.  Now that it does, it should help some.
> 
> The Fedora BZ, if you're interested, is here:
> 
>    https://bugzilla.redhat.com/show_bug.cgi?id=1219341
> 
> 
> On 06/03/2015 08:57 AM, Moore, Robert wrote:
> > And of course, the warning message should be taken to heart:
> >
> >     /*
> >      * iASL Warning: There were 4 external control methods found during
> >      * disassembly, but additional ACPI tables to resolve these
> externals
> >      * were not specified. This resulting disassembler output file may
> not
> >      * compile because the disassembler did not know how many arguments
> >      * to assign to these methods. To specify the tables needed to
> resolve
> >      * external control method references, the -e option can be used to
> >      * specify the filenames. Note: SSDTs can be dynamically loaded at
> >      * runtime and may or may not be available via the host OS.
> >      * Example iASL invocations:
> >      *     iasl -e ssdt1.aml ssdt2.aml ssdt3.aml -d dsdt.aml
> >      *     iasl -e dsdt.aml ssdt2.aml -d ssdt1.aml
> >      *     iasl -e ssdt*.aml -d dsdt.aml
> >      *
> >      * In addition, the -fe option can be used to specify a file
> containing
> >      * control method external declarations with the associated method
> >      * argument counts. Each line of the file must be of the form:
> >      *     External (<method pathname>, MethodObj, <argument count>)
> >      * Invocation:
> >      *     iasl -fe refs.txt -d dsdt.aml
> >      *
> >      * The following methods were unresolved and many not compile
> properly
> >      * because the disassembler had to guess at the number of arguments
> >      * required for each:
> >      */
> >
> >
> >> -----Original Message-----
> >> From: Zheng, Lv
> >> Sent: Tuesday, June 02, 2015 5:38 PM
> >> To: Moore, Robert; Al Stone; devel(a)acpica.org
> >> Subject: RE: [Devel] [RFC] proposed patch for some segfaults
> >> encountered
> >>
> >> Hi,
> >>
> >>> From: Devel [mailto:devel-bounces(a)acpica.org] On Behalf Of Moore,
> >>> Robert
> >>> Sent: Wednesday, June 03, 2015 4:06 AM
> >>>
> >>> It looks to me that the disassembler has to start guessing at the
> >>> number of arguments for the various external control methods, and it
> >> begins to get seriously confused.
> >>>
> >>> Just FYI, here is a full description of the problem:
> >>>
> >>> 4.4.1	Disassembly of Control Method Invocations
> >>>
> >>> Summary of the external control method problem:
> >>> When the -e option is used with disassembly, the various SSDTs are
> >>> simply loaded into a global namespace for the disassembler to use in
> >> order to resolve control method references (invocations).
> >>> The disassembler tracks any such references, and will emit an
> >>> External() statement for these types of methods, with the proper
> >>> number
> >> of arguments .
> >>> Without the SSDTs, the AML does not contain enough information to
> >>> properly disassemble the control method invocation -- because the
> >> disassembler does not know how many arguments to parse.
> >>> An example: Assume we have two control methods. ABCD has one
> >>> argument, and EFGH has zero arguments. Further, we have two
> >>> additional control
> >> methods that invoke ABCD and EFGH, named T1 and T2:
> >>>
> >>>     Method (ABCD, 1)
> >>>     {
> >>>     }
> >>>     Method (EFGH, 0)
> >>>     {
> >>>     }
> >>>     Method (T1)
> >>>     {
> >>>         ABCD (Add (2, 7, Local0))
> >>>     }
> >>>     Method (T2)
> >>>     {
> >>>         EFGH ()
> >>>         Add (2, 7, Local0)
> >>>     }
> >>>
> >>> Here is the AML code that is generated for T1 and T2:
> >>>
> >>>      185:      Method (T1)
> >>>
> >>> 0000034C:  14 10 54 31 5F 5F 00 ...    "..T1__."
> >>>
> >>>      186:      {
> >>>      187:          ABCD (Add (2, 7, Local0))
> >>>
> >>> 00000353:  41 42 43 44 ............    "ABCD"
> >>> 00000357:  72 0A 02 0A 07 60 ......    "r....`"
> >>>
> >>>      188:      }
> >>>
> >>>      190:      Method (T2)
> >>>
> >>> 0000035D:  14 10 54 32 5F 5F 00 ...    "..T2__."
> >>>
> >>>      191:      {
> >>>      192:          EFGH ()
> >>>
> >>> 00000364:  45 46 47 48 ............    "EFGH"
> >>>
> >>>      193:          Add (2, 7, Local0)
> >>>
> >>> 00000368:  72 0A 02 0A 07 60 ......    "r....`"
> >>>      194:      }
> >>>
> >>> Note that the AML code for T1 and T2 is essentially identical. When
> >>> disassembling this code, the methods ABCD and EFGH must be known to
> >>> the
> >> disassembler, otherwise it does not know how to handle the method
> >> invocations.
> >>
> >> This is the problem in ACPI specification since DSDT 1.0:
> >> MethodInvocation := NameString TermArgList TermObj :=
> >> NameSpaceModifierObj
> >> | NamedObj | Type1Opcode | Type2Opcode TermList := Nothing | <TermObj
> >> TermList> There is no number of arguments defined in the grammer.
> >>
> >>> In other words, if ABCD and EFGH are actually external control
> >>> methods appearing in an SSDT, the disassembler does not know what to
> >>> do unless
> >> the owning SSDT has been loaded via the -e option.
> >>
> >> Normally  I will use the following command to decompile:
> >> # iasl -e dsdt.dat -e ssdt*.dat -d ssdt9.dat Which loads all tables
> >> in the same order as the OSPM (may still face issue if there is
> "Load/LoadTable"
> >> invoked during the table loading).
> >> If this still failed, this was either a BIOS bug or an DSDT 2.0
> >> compatible problem.
> >>
> >> Thanks and best regards
> >> -Lv
> >>
> >>>
> >>> 4.4.1.1	ACPI 6.0 Solution - External() AML Opcode
> >>> In ACPI 6.0, a new AML opcode was defined to help with this problem.
> >>> The opcode is not intended for execution by AML interpreters; its
> >>> only
> >> purpose is to assist the disassembler with the control method
> >> argument count problem.
> >>>
> >>>
> >>>
> >>>
> >>>> -----Original Message-----
> >>>> From: Devel [mailto:devel-bounces(a)acpica.org] On Behalf Of Moore,
> >>>> Robert
> >>>> Sent: Tuesday, June 02, 2015 12:06 PM
> >>>> To: Al Stone; devel(a)acpica.org
> >>>> Subject: Re: [Devel] [RFC] proposed patch for some segfaults
> >>>> encountered
> >>>>
> >>>> I'll need to disassemble with the DSDT and all SSDTs.
> >>>>
> >>>> Do you have an acpidump for the machine? Otherwise, the binary
> >>>> files will be ok.
> >>>>
> >>>> There are a lot of externals to other tables, and this may be a
> >>>> part of the problem.
> >>>>
> >>>>
> >>> _______________________________________________
> >>> Devel mailing list
> >>> Devel(a)acpica.org
> >>> https://lists.acpica.org/mailman/listinfo/devel
> 
> 
> --
> ciao,
> al
> -----------------------------------
> Al Stone
> Software Engineer
> Red Hat, Inc.
> ahs3(a)redhat.com
> -----------------------------------

Re: [Devel] [RFC] proposed patch for some segfaults encountered

[Moore, Robert]

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

Anyway, I added a check for a null target in AcpiDmIsValidTarget, and for the StoreOp target case. That should at least cover all of these types of cases.


> -----Original Message-----
> From: Devel [mailto:devel-bounces(a)acpica.org] On Behalf Of Al Stone
> Sent: Monday, June 01, 2015 1:59 PM
> To: devel(a)acpica.org
> Subject: [Devel] [RFC] proposed patch for some segfaults encountered
> 
> It's not clear to me that this is the correct way to handle a segfault
> that was reported to me via Fedora.
> 
> Using the ssdt9.dat file attached, a simple "iasl -d ssdt9.dat" will
> segfault around calls to AcpiDmPromoteTarget -- and it turns out that the
> Target pointer being passed into that function was sometimes null but not
> being checked in the calling function, AcpiDmCheckForSymbolicOpcode.  So,
> this patch adds in some checks for null pointers and can now get through
> this particular problem.
> 
> However, I did not study AcpiDmCheckForSymbolicOpcode long enough to
> determine if the Target pointer in that function should or should not ever
> be null.  So, this patch fixes this issue, but it may not be the proper
> long term solution.
> 
> --
> ciao,
> al
> -----------------------------------
> Al Stone
> Software Engineer
> Red Hat, Inc.
> ahs3(a)redhat.com
> -----------------------------------

Re: [Devel] [RFC] proposed patch for some segfaults encountered

[Al Stone]

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

Awesome.  Thanks.

On 06/03/2015 03:06 PM, Moore, Robert wrote:
> Anyway, I added a check for a null target in AcpiDmIsValidTarget, and for the StoreOp target case. That should at least cover all of these types of cases.
> 
> 
>> -----Original Message-----
>> From: Devel [mailto:devel-bounces(a)acpica.org] On Behalf Of Al Stone
>> Sent: Monday, June 01, 2015 1:59 PM
>> To: devel(a)acpica.org
>> Subject: [Devel] [RFC] proposed patch for some segfaults encountered
>>
>> It's not clear to me that this is the correct way to handle a segfault
>> that was reported to me via Fedora.
>>
>> Using the ssdt9.dat file attached, a simple "iasl -d ssdt9.dat" will
>> segfault around calls to AcpiDmPromoteTarget -- and it turns out that the
>> Target pointer being passed into that function was sometimes null but not
>> being checked in the calling function, AcpiDmCheckForSymbolicOpcode.  So,
>> this patch adds in some checks for null pointers and can now get through
>> this particular problem.
>>
>> However, I did not study AcpiDmCheckForSymbolicOpcode long enough to
>> determine if the Target pointer in that function should or should not ever
>> be null.  So, this patch fixes this issue, but it may not be the proper
>> long term solution.
>>
>> --
>> ciao,
>> al
>> -----------------------------------
>> Al Stone
>> Software Engineer
>> Red Hat, Inc.
>> ahs3(a)redhat.com
>> -----------------------------------


-- 
ciao,
al
-----------------------------------
Al Stone
Software Engineer
Red Hat, Inc.
ahs3(a)redhat.com
-----------------------------------