Most recent kernel where this bug did not occur: Distribution: Hardware Environment: Software Environment: Problem Description: The Linux kernel ACPI interpreter fails the following AML test when it is compiled into a simulator. So if an OEM BIOS includes this code, Linux would fail. Steps to reproduce: Run interpreter with the .asl demo code attached below.
Created attachment 8042 [details] ASL code to reproduce bug ACPICA AML interpreter (acpiexec) should be ran with -s flag (slack mode). Output of the demo: Executing \MAIN [ACPI Debug] Integer: 0x00000000000000F1 Outstanding: 0x1 allocations after execution Execution of \MAIN returned object 00326E38 Buflen 10 [Integer] = 00000000000000F1
DESCRIPTION: In ACPICA AML interpreter Slack mode outstanding allocations are detected when AML-code looks like the following lines: Method (SBYT, 1) { Store (Arg0, Debug) } Method(MAIN) { Or (0xf0, 0x01, Local0) SBYT (Local0) } The problem was initially held up by FreeBSD: > -----Original Message----- > From: Nate Lawson [mailto:nate@root.org] > Sent: Monday, February 13, 2006 2:16 PM > To: Jung-uk Kim > Cc: Moore, Robert > Subject: Re: ACPI-CA memory leak > > Judging from your patch, my thought is that the store from Local1 to > DATS is not being reference-counted properly and so the local object is > never freed. The above proposed example reproduces a very probable way of that case. Both optional storing to Local0 and manipulation with it in a called Method (SBYT) are essential here. Note: there is no any outstanding allocations when Store (Or (0xf0, 0x01), Local0) is used instead of Or(0xf0, 0x01, Local0). Looks like the following code in the function AcpiDsDoImplicitReturn (file interpreter/dispatcher/dsutils.c) takes effect in our case: /* * Delete any "stale" implicit return value first. However, in * complex statements, the implicit return value can be * bubbled up several levels, so we don't clear the value if it * is the same as the ReturnDesc. */ if (WalkState->ImplicitReturnObj) { if (WalkState->ImplicitReturnObj == ReturnDesc) { return (TRUE); } AcpiDsClearImplicitReturn (WalkState); } The RefCount of the Local0 object (from the example) incremented for MAIN--WalkState->ImplicitReturnObj was additionally incremented for the terminated method SBYT--WalkState->ImplicitReturnObj (ReturnDesc). So, after returning from the called method (SBYT) to the current method (MAIN) the previous MAIN--WalkState->ImplicitReturnObj should explicitly be cleared through the call to AcpiDsClearImplicitReturn. The following patch can solve this problem: --- dsutils.c.orig 2006-03-28 12:48:09.146714200 +0400 +++ dsutils.c 2006-03-28 12:48:37.087169200 +0400 @@ -228,8 +228,4 @@ if (WalkState->ImplicitReturnObj) { - if (WalkState->ImplicitReturnObj == ReturnDesc) - { - return (TRUE); - } AcpiDsClearImplicitReturn (WalkState); } Although ASL tests are not specifically designed to cover Slack mode issues, this problem evidently is touched by them. There are different numbers of outstanding allocations on Slack and non-Slack mode. After this patch some Slack numbers became less than before. However there is no certainty now that the proposed update is the exact solution of the problem. This bug should be additionally investigated along with other events of outstanding allocations. INTERNAL BUG NUMBER: 211
Additional consideration. The same result (outstanding allocation) takes place if explicit Return operator will be used in SBYT method above: Method (SBYT, 1) { - Store (Arg0, Debug) + Return (Arg0) } Evidently the RefCount of the original Local0 object (passed into SBYT) incremented for SBYT--WalkState->ReturnDesc gives the same negative effect. The mentioned above fragment of AcpiDsDoImplicitReturn resolves Implicit Return processing issue when the same Object could become the Result Object of several successive AML operators ("bubbled up several levels"). In that case would be superfluous decrementing Object's RefCount (by means AcpiDsClearImplicitReturn) to increment it then: /* Save the implicit return value, add a reference if requested */ WalkState->ImplicitReturnObj = ReturnDesc; if (AddReference) { AcpiUtAddReference (ReturnDesc); } The AddReference parameter of AcpiDsDoImplicitReturn is FALSE above when this function is invoked after an AML method return so that RefCount of the previous Implicit Return Object is decremented and can become Zero (the Object will be deleted if any) but RefCount of the new Implicit Return Object will not be incremented. This is right for a called method Return Object in the most cases but does not work when it is the same as the previous Implicit Return Object. That additional issue could be fixed by the following patch: diff -r -u source3/components/interpreter/dispatcher/dsutils.c source/components/interpreter/dispatcher/dsutils.c --- source3/components/interpreter/dispatcher/dsutils.c 2006-04-24 12:25:15.000000000 +0400 +++ source/components/interpreter/dispatcher/dsutils.c 2006-05-06 11:00:06.979158800 +0400 @@ -228,6 +228,10 @@ { if (WalkState->ImplicitReturnObj == ReturnDesc) { + if (!AddReference) + { /* Delete additional reference corresponding to ReturnDesc */ + AcpiUtRemoveReference (ReturnDesc); + } return (TRUE); } AcpiDsClearImplicitReturn (WalkState);
Created attachment 8043 [details] Proposed patch The update was tested by the ASLTS test suite runs (all the test cases) on the following systems: - CYGWIN_NT-5.1 1.5.18(0.132/4/2) 2005-07-02 20:30 i686 unknown unknown Cygwin - Linux 2.6.17-rc1-mm2 #1 SMP Thu Apr 13 14:03:28 MSD 2006 i686 i686 i386 GNU/Linux for the following (all) modes supported by the ASLTS test suite: - 32-bit norm mode - 64-bit norm mode - 32-bit slack mode - 32-bit slack mode The tests were run (on the systems described above) before update, then the tests were run on those systems after update. Then results of two runs were compared. All is Ok (the numbers of outstanding allocations after update in norm mode are the same and in slack mode are equal to or less than before update, no new errors).
Additional consideration 2. Actually a relevant to the proposed patch action is provided for normal Return Object processing in the function AcpiDsRestartControlMethod (file interpreter/dispatcher/dsmethod.c) if "the return value will not be used by the calling method": if (WalkState->ReturnUsed) { /* Save the return value from the previous method */ ... else if (!AcpiDsDoImplicitReturn (ReturnDesc, WalkState, FALSE)) { /* * Delete the return value if it will not be used by the * calling method */ AcpiUtRemoveReference (ReturnDesc); } So the following patch concerned with AcpiDsRestartControlMethod and resolving the problem too looks like more preferable: diff -r -u source3/components/interpreter/dispatcher/dsmethod.c source/components/interpreter/dispatcher/dsmethod.c --- source3/components/interpreter/dispatcher/dsmethod.c 2006-04-24 12:25:14.000000000 +0400 +++ source/components/interpreter/dispatcher/dsmethod.c 2006-05-08 14:36:20.791169200 +0400 @@ -492,6 +492,7 @@ ACPI_OPERAND_OBJECT *ReturnDesc) { ACPI_STATUS Status; + UINT32 IsImplicitSelfsame; ACPI_FUNCTION_TRACE_PTR (DsRestartControlMethod, WalkState); @@ -511,6 +512,9 @@ if (ReturnDesc) { + /* Opportunely calculate Implicit Return specific condition */ + IsImplicitSelfsame = (WalkState->ImplicitReturnObj == ReturnDesc); + /* Are we actually going to use the return value? */ if (WalkState->ReturnUsed) @@ -539,7 +543,12 @@ * NOTE: this is optional because the ASL language does not actually * support this behavior. */ - else if (!AcpiDsDoImplicitReturn (ReturnDesc, WalkState, FALSE)) + else if (!AcpiDsDoImplicitReturn (ReturnDesc, WalkState, FALSE) || + /* + * Take into account "complex statements" + * processing in AcpiDsDoImplicitReturn + */ + IsImplicitSelfsame) { /* * Delete the return value if it will not be used by the
Created attachment 8044 [details] Proposed patch 2
Essentials of patch 2 integrated and released in ACPICA version 20060512
ACPICA 20060608 shipped in 2.6.17-git9, closed.
*** Bug 7163 has been marked as a duplicate of this bug. ***
re-opening because we need a version of this patch for 2.6.16/2.6.17
Created attachment 9059 [details] Patch for 2.6.16/2.6.17 Extracted from patch-2.6.18.rc1
Thanks for also adding the backport. It's very much appreciated! If there pop up similar things, it would be nice if you can add me into CC list if I am not already. It's very easy to oversee such important fixes. Thanks.
Fixed since 2.6.18, patch for older kernels available.