Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for github issue 659 #663

Merged
merged 2 commits into from Feb 18, 2021
Merged

Fix for github issue 659 #663

merged 2 commits into from Feb 18, 2021

Conversation

SchmErik
Copy link
Contributor

@SchmErik SchmErik commented Feb 2, 2021

This pull request contains fixes for github issue 659 by changing iASL behavior to keep external methods that are referenced in iASL constructs such as if (CondRefOf (EXT0))

This change prepares ExInsertArgCount so that a function that searches
the AslGbl_ExternalList can be extracted.

Signed-off-by: Erik Kaneda <erik.kaneda@intel.com>
@1alessandro1
Copy link

First, thanks for the PR and for your time and dedication

However, I compiled the latest binaries with this PR merged and the issue persists.
I'll contact foskvs if it's the same on his end

@SchmErik
Copy link
Contributor Author

SchmErik commented Feb 2, 2021

@1alessandro1 could you post the file that exhibits this issue? I tried to compile the tables posted by @foskvs in the issues page but I'm not seeing the compiler error.

@SchmErik
Copy link
Contributor Author

SchmErik commented Feb 2, 2021

I was a little confused by what @foskvs said but I see the compiler error now. I've fixed the compiler so that it can allow iASL to compile, disassemble, and recompile the code in question. However, if you're doing the disassembly and re-compilation on tables that have been initially compiled other than this version of the compiler, then you'll still get the error because the external method declarations have been omitted in the initial compilation.

At the moment, the external analysis walk attempts to optimize the size
of AML by removing external method declarations that are not called.
This results in the external declaration in the following code being
removed during compilation:

External (EXT0, MethodObj) // EXT0 removed during compilation
if (!CondRefOf (EXT0))
{
    Method (EXT0)
    {
        ...
    }
}

This happens because referencing externals inside of CondRefOf is not a
method call. This results in the first line being removed during
compilation and breaks the re-compilation of the above code after
disassembly. In order to retain the external declaration, change the
external method analysis to look for any named referenced in addition to
method calls.

Signed-off-by: Erik Kaneda <erik.kaneda@intel.com>
@1alessandro1
Copy link

@SchmErik Please test this (compile, no errors, then decompile, and recompile)

SSDT-EC-USBX-DESKTOP.dsl.zip

@SchmErik
Copy link
Contributor Author

SchmErik commented Feb 2, 2021

ah, I made a slight mistake and I fixed it just now. Could you try fetching and re-compiling? I force-pushed so you might need to do something like git reset --hard

@1alessandro1
Copy link

perfect, the issue is gone

@acpibob acpibob merged commit 52a8236 into acpica:master Feb 18, 2021
SchmErik pushed a commit to SchmErik/acpica that referenced this pull request Mar 2, 2021
This reverts commit 52a8236, reversing
changes made to 79acf21.

This pull request was intended to resolve a bug in the compiler but
further exposed a different bug in the compiler. Revert the material
from this pull request for now in order to diagnost the issue. It would
be nice to come up with solutions that simplify the logic in iASL's
external analysis.

Signed-off-by: Erik Kaneda <erik.kaneda@intel.com>
1alessandro1 added a commit to 1alessandro1/acpica that referenced this pull request Mar 10, 2021
Revert "Merge pull request acpica#663 from SchmErik/ghi-659"
acpibob added a commit that referenced this pull request Mar 12, 2021
Revert "Merge pull request #663 from SchmErik/ghi-659"
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants