Bug 5358 - Store of Index reference to another element of the same Package causes hang
Summary: Store of Index reference to another element of the same Package causes hang
Status: REJECTED WILL_NOT_FIX
Alias: None
Product: ACPI
Classification: Unclassified
Component: ACPICA-Core (show other bugs)
Hardware: i386 Linux
: P2 normal
Assignee: Lin Ming
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2005-10-04 06:10 UTC by Valery A Podrezov
Modified: 2008-06-27 22:17 UTC (History)
1 user (show)

See Also:
Kernel Version: 2.6
Subsystem:
Regression: ---
Bisected commit-id:


Attachments
ASL code to reproduce bug. (583 bytes, text/plain)
2005-10-04 06:16 UTC, Valery A Podrezov
Details
Proposed patch (38.37 KB, patch)
2006-04-06 11:46 UTC, Valery A Podrezov
Details | Diff
Proposed patch (38.03 KB, patch)
2006-04-14 12:32 UTC, Valery A Podrezov
Details | Diff
Proposed patch (39.21 KB, patch)
2006-04-25 06:14 UTC, Valery A Podrezov
Details | Diff
patch for comment #7 (1.29 KB, patch)
2007-10-26 01:29 UTC, Lin Ming
Details | Diff
This patch follows reference semantic (3.36 KB, patch)
2007-10-30 17:54 UTC, Lin Ming
Details | Diff

Description Valery A Podrezov 2005-10-04 06:10:30 UTC
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.
Comment 1 Valery A Podrezov 2005-10-04 06:16:11 UTC
Created attachment 6214 [details]
ASL code to reproduce bug.


Store of Index reference to 0-th element of Package into 1-th element
of the same Package causes hang.

INTERNAL BUG NUMBER

   135
Comment 2 Valery A Podrezov 2006-04-06 11:46:46 UTC
Created attachment 7789 [details]
Proposed patch

TESTED:


The additional tests were designed to verify this update
comprehensively. All the 'outstanding allocations' reported
after execution of these tests have been extirpated (no one
now on the mentioned tests). It was quite complicated task
to fix this bug and pass through the merciless new designed
tests without 'outstanding allocations'.


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.5-7.191-smp #1 SMP Tue Jun 28 14:58:56 UTC 2005 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
- 64-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 relevant error fixed, no
new errors).
Comment 3 Valery A Podrezov 2006-04-14 12:32:02 UTC
Created attachment 7873 [details]
Proposed patch

Generalization is made:

- to support Buffer and String type parents of Index Reference
- in accordance with the new bug 6389(212) I see the updated routines
  will be applied for RefOf-reference case also, so identifiers
  and comments are freed from specifications 'Index Reference'.

The update was tested as usually by the ASLTS test suite runs
on both Cygwin and Linux systems.
Comment 4 Valery A Podrezov 2006-04-25 06:14:43 UTC
Created attachment 7948 [details]
Proposed patch

Generalization: changed PkgByArgs to ArgState to trace the state
of all type objects assigned to Args, not only Packages.
Comment 5 Valery A Podrezov 2007-01-23 10:30:47 UTC
Bug will be resolved in consequence of fixing another bug
Comment 6 Lin Ming 2007-10-25 20:14:29 UTC
The root cause is "recursive adding reference", for example,
Name(p000, Package() {1,2,3,4,5,6,7,8,9})
Store(Index(p000, 0), Index(p000, 1))

In this Store operation, would firstly copy a Refrence Object(NewRefObj) for Index(p000, 0), and then store NewRefObj to Index(p000, 1)
After this Store, p000 becomes Package() {1,NewRefObj,3,4,5,6,7,8,9}

So, if add refrence to p000, the recursion occurs, see below,
AddReference(p000)
AddReference(Index(p000, 0))
AddReference(Index(p000, 1)) -> Index(p000, 1)=NewRefObj, reference to p000
        AddReference(p000)   -> recursion
        AddReference(Index(p000, 0))
        AddReference(Index(p000, 1))
   
Comment 7 Lin Ming 2007-10-25 20:19:44 UTC
Name(p000, Package() {1,2,3,4,5,6,7,8,9})
Store(Index(p000, 0), Index(p000, 1))

In this case, maybe we should firstly dereference Index(p000, 0), 
and then store the dereferenced value to Index(p000, 1) instead of the reference itself.

Any idea Bob?
Comment 8 Lin Ming 2007-10-26 01:29:17 UTC
Created attachment 13283 [details]
patch for comment #7

For operation Store(RefA, RefB)
    
Dereference RefA and then store the deferenced value to RefB
Comment 9 Lin Ming 2007-10-29 19:35:04 UTC
Comment #8 changes the Reference Semantic, need to refresh the patch

Name(p000, Package() {1,2,3,4,5,6,7,8,9})
Store(Index(p000, 0), Index(p000, 1))

If we store another value to Index(p000, 0), Index(p000, 1) should also be changed, since it's a reference.

Store("hello", Index(p000, 0))
the value in Index(p000, 1) should also be "hello"
Comment 10 Lin Ming 2007-10-30 17:54:14 UTC
Created attachment 13356 [details]
This patch follows reference semantic

Tested against ASLTS, it works and no regression.

With this patch applied, when store an Index Reference to another element of the same package, it would only add a reference count to this Index Reference Object, and not add reference to the package, so it can prevent the recursion.
Comment 11 Len Brown 2008-01-09 01:07:54 UTC
is this included in a version of ACPICA?
Comment 12 Lin Ming 2008-01-09 17:21:01 UTC
Not yet.
Comment 13 Len Brown 2008-06-13 20:12:46 UTC
lin-ming, what is the state of this sighting?
Comment 14 Lin Ming 2008-06-25 18:56:41 UTC
This is an academic bug.

Patch available, but we are not going to integrate it.

Note You need to log in before you can comment on or make changes to this bug.