Skip to content

Commit

Permalink
iASL/Disassembler: Improve handling of unresolved methods
Browse files Browse the repository at this point in the history
If the definition of a control method cannot be found (probably it
is in another module/SSDT), the disassembler must try to guess
at the number of arguments to that method. This change improves
the guessing heuristic.
  • Loading branch information
acpibob committed Mar 17, 2016
1 parent b562520 commit 16cd087
Show file tree
Hide file tree
Showing 4 changed files with 109 additions and 100 deletions.
81 changes: 56 additions & 25 deletions source/common/adwalk.c
Expand Up @@ -515,7 +515,7 @@ AcpiDmDumpDescending (
*
* DESCRIPTION: Check namepath Ops for orphaned method invocations
*
* Note: Experimental.
* Note: Parts of this are experimental, under possible further development.
*
******************************************************************************/

Expand Down Expand Up @@ -581,6 +581,7 @@ AcpiDmFindOrphanDescending (
ChildOp->Common.Value.String, ACPI_TYPE_METHOD, ArgCount, 0);
}
break;

#endif

case AML_STORE_OP:
Expand All @@ -605,7 +606,7 @@ AcpiDmFindOrphanDescending (
/* One Arg means this is just a Store(Name,Target) */

AcpiDmAddOpToExternalList (ChildOp,
ChildOp->Common.Value.String, ACPI_TYPE_INTEGER, 0, 0);
ChildOp->Common.Value.String, ACPI_TYPE_INTEGER, ArgCount, 0);
return (AE_OK);
}

Expand All @@ -627,7 +628,7 @@ AcpiDmFindOrphanDescending (
(ParentOp->Common.AmlOpcode != AML_INT_METHODCALL_OP) &&
!Op->Common.Node)
{
ArgCount = AcpiDmInspectPossibleArgs (0, 0, Op->Common.Next);
ArgCount = AcpiDmInspectPossibleArgs (0, 0, Op);

/*
* Check if namepath is a predicate for if/while or lone parameter to
Expand Down Expand Up @@ -861,7 +862,7 @@ AcpiDmXrefDescendingOp (
Op->Common.Next->Common.Next->Common.Value.Integer;
}

Flags = Flags | ACPI_EXT_RESOLVED_REFERENCE;
Flags |= ACPI_EXT_RESOLVED_REFERENCE | ACPI_EXT_ORIGIN_FROM_OPCODE;
AcpiDmAddOpToExternalList (Op, Path,
(UINT8) ObjectType, ParamCount, Flags);

Expand Down Expand Up @@ -929,6 +930,7 @@ AcpiDmXrefDescendingOp (
Status = AcpiNsLookup (WalkState->ScopeInfo, Path, ACPI_TYPE_ANY,
ACPI_IMODE_EXECUTE, ACPI_NS_SEARCH_PARENT | ACPI_NS_DONT_OPEN_SCOPE,
WalkState, &Node);

if (ACPI_SUCCESS (Status) && (Node->Flags & ANOBJ_IS_EXTERNAL))
{
/* Node was created by an External() statement */
Expand All @@ -953,12 +955,12 @@ AcpiDmXrefDescendingOp (
if (Node)
{
AcpiDmAddNodeToExternalList (Node,
(UINT8) ObjectType, 0, Flags);
(UINT8) ObjectType, 7, Flags);
}
else
{
AcpiDmAddOpToExternalList (Op, Path,
(UINT8) ObjectType, 0, Flags);
(UINT8) ObjectType, 7, Flags);
}
}
}
Expand Down Expand Up @@ -1140,40 +1142,69 @@ AcpiDmInspectPossibleArgs (
{
const ACPI_OPCODE_INFO *OpInfo;
UINT32 i;
UINT32 Last = 0;
UINT32 Lookahead;
UINT32 ArgumentCount = 0;
ACPI_PARSE_OBJECT *NextOp;
ACPI_PARSE_OBJECT *ExecuteOp;


Lookahead = (ACPI_METHOD_NUM_ARGS + TargetCount) - CurrentOpArgCount;
if (!Op)
{
return (0);
}

/* Lookahead for the maximum number of possible arguments */

for (i = 0; i < Lookahead; i++)
NextOp = Op->Common.Next;

for (i = 0; (i < ACPI_METHOD_NUM_ARGS) && NextOp; i++)
{
if (!Op)
{
break;
}
OpInfo = AcpiPsGetOpcodeInfo (NextOp->Common.AmlOpcode);

OpInfo = AcpiPsGetOpcodeInfo (Op->Common.AmlOpcode);
/* Any one of these operators is "very probably" not a method arg */

/*
* Any one of these operators is "very probably" not a method arg
*/
if ((Op->Common.AmlOpcode == AML_STORE_OP) ||
(Op->Common.AmlOpcode == AML_NOTIFY_OP))
if ((NextOp->Common.AmlOpcode == AML_STORE_OP) ||
(NextOp->Common.AmlOpcode == AML_NOTIFY_OP) ||
(OpInfo->Class == AML_CLASS_CONTROL) ||
(OpInfo->Class == AML_CLASS_CREATE) ||
(OpInfo->Class == AML_CLASS_NAMED_OBJECT))
{
break;
}

if ((OpInfo->Class != AML_CLASS_EXECUTE) &&
(OpInfo->Class != AML_CLASS_CONTROL))
if (OpInfo->Class == AML_CLASS_EXECUTE)
{
Last = i+1;
/* Probable that this is method arg if there is no target */

ExecuteOp = NextOp->Common.Value.Arg;
while (ExecuteOp)
{
if ((ExecuteOp->Common.AmlOpcode == AML_INT_NAMEPATH_OP) &&
(ExecuteOp->Common.Value.Arg == NULL))
{
/* No target, could be a method arg */

break;
}

if (NextOp->Common.AmlOpcode == AML_REF_OF_OP)
{
break;
}

ExecuteOp = ExecuteOp->Common.Next;
}

if (!ExecuteOp)
{
/* Has a target, not method arg */

return (ArgumentCount);
}
}

Op = Op->Common.Next;
ArgumentCount++;
NextOp = NextOp->Common.Next;
}

return (Last);
return (ArgumentCount);
}
122 changes: 49 additions & 73 deletions source/common/dmextern.c
Expand Up @@ -884,24 +884,27 @@ AcpiDmCreateNewExternal (

if (!strcmp (ExternalPath, NextExternal->Path))
{
/* Duplicate method, check that the Value (ArgCount) is the same */

if ((NextExternal->Type == ACPI_TYPE_METHOD) &&
(NextExternal->Flags & ANOBJ_IS_EXTERNAL) &&
(NextExternal->Value != Value) &&
(Value > 0))
/*
* If this external came from an External() opcode, we are
* finished with this one. (No need to check any further).
*/
if (NextExternal->Flags & ACPI_EXT_ORIGIN_FROM_OPCODE)
{
ACPI_ERROR ((AE_INFO,
"External method arg count mismatch %s: "
"Current %u, attempted %u",
NextExternal->Path, NextExternal->Value, Value));
return_ACPI_STATUS (AE_ALREADY_EXISTS);
}

/* Allow upgrade of type from ANY */

else if (NextExternal->Type == ACPI_TYPE_ANY)
else if ((NextExternal->Type == ACPI_TYPE_ANY) &&
(Type != ACPI_TYPE_ANY))
{
NextExternal->Type = Type;
}

/* Update the argument count as necessary */

if (Value < NextExternal->Value)
{
NextExternal->Value = Value;
}

Expand Down Expand Up @@ -1157,90 +1160,63 @@ AcpiDmEmitExternals (

AcpiDmUnresolvedWarning (1);

/* Emit any unresolved method externals in a single text block */

NextExternal = AcpiGbl_ExternalList;
while (NextExternal)
{
if ((NextExternal->Type == ACPI_TYPE_METHOD) &&
(!(NextExternal->Flags & ACPI_EXT_RESOLVED_REFERENCE)))
{
AcpiOsPrintf (" External (%s%s",
NextExternal->Path,
AcpiDmGetObjectTypeName (NextExternal->Type));

AcpiOsPrintf (") // Warning: Unresolved method, "
"guessing %u arguments\n",
NextExternal->Value);

NextExternal->Flags |= ACPI_EXT_EXTERNAL_EMITTED;
}

NextExternal = NextExternal->Next;
}

AcpiOsPrintf ("\n");


/* Emit externals that were imported from a file */

if (Gbl_ExternalRefFilename)
{
AcpiOsPrintf (
" /*\n * External declarations that were imported from\n"
" * the reference file [%s]\n */\n",
" /*\n * External declarations were imported from\n"
" * a reference file -- %s\n */\n\n",
Gbl_ExternalRefFilename);

NextExternal = AcpiGbl_ExternalList;
while (NextExternal)
{
if (!(NextExternal->Flags & ACPI_EXT_EXTERNAL_EMITTED) &&
(NextExternal->Flags & ACPI_EXT_ORIGIN_FROM_FILE))
{
AcpiOsPrintf (" External (%s%s",
NextExternal->Path,
AcpiDmGetObjectTypeName (NextExternal->Type));

if (NextExternal->Type == ACPI_TYPE_METHOD)
{
AcpiOsPrintf (") // %u Arguments\n",
NextExternal->Value);
}
else
{
AcpiOsPrintf (")\n");
}
NextExternal->Flags |= ACPI_EXT_EXTERNAL_EMITTED;
}

NextExternal = NextExternal->Next;
}

AcpiOsPrintf ("\n");
}

/*
* Walk the list of externals found during the AML parsing
* Walk and emit the list of externals found during the AML parsing
*/
while (AcpiGbl_ExternalList)
{
AcpiGbl_ExternalList = AcpiGbl_ExternalList;

This comment has been minimized.

Copy link
@juikim

juikim Mar 18, 2016

Contributor

This change looks wrong. At least, it looks redundant.

This comment has been minimized.

Copy link
@acpibob

acpibob via email Mar 18, 2016

Author Contributor

This comment has been minimized.

Copy link
@juikim

juikim Mar 18, 2016

Contributor

AcpiGbl_ExternalList = AcpiGbl_ExternalList is no-op. Actually, clang complains and fails:

../../../source/common/dmextern.c:1104:30: error: explicitly assigning value of variable of type 'ACPI_EXTERNAL_LIST *'
      (aka 'struct acpi_external_list *') to itself [-Werror,-Wself-assign]
        AcpiGbl_ExternalList = AcpiGbl_ExternalList;
        ~~~~~~~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~
1 error generated.

This comment has been minimized.

Copy link
@acpibob

acpibob via email Mar 18, 2016

Author Contributor
if (!(AcpiGbl_ExternalList->Flags & ACPI_EXT_EXTERNAL_EMITTED))
{
AcpiOsPrintf (" External (%s%s",
AcpiOsPrintf (" External (%s%s)",
AcpiGbl_ExternalList->Path,
AcpiDmGetObjectTypeName (AcpiGbl_ExternalList->Type));

/* For methods, add a comment with the number of arguments */
/* Check for "unresolved" method reference */

if (AcpiGbl_ExternalList->Type == ACPI_TYPE_METHOD)
if ((AcpiGbl_ExternalList->Type == ACPI_TYPE_METHOD) &&
(!(AcpiGbl_ExternalList->Flags & ACPI_EXT_RESOLVED_REFERENCE)))
{
AcpiOsPrintf (") // %u Arguments\n",
AcpiOsPrintf (" // Warning: Unknown method, "
"guessing %u arguments",
AcpiGbl_ExternalList->Value);
}

/* Check for external from a external references file */

else if (AcpiGbl_ExternalList->Flags & ACPI_EXT_ORIGIN_FROM_FILE)
{
if (AcpiGbl_ExternalList->Type == ACPI_TYPE_METHOD)
{
AcpiOsPrintf (" // %u Arguments",
AcpiGbl_ExternalList->Value);
}

AcpiOsPrintf (" // From external reference file");
}

/* This is the normal external case */

else
{
AcpiOsPrintf (")\n");
/* For methods, add a comment with the number of arguments */

if (AcpiGbl_ExternalList->Type == ACPI_TYPE_METHOD)
{
AcpiOsPrintf (" // %u Arguments",
AcpiGbl_ExternalList->Value);
}
}

AcpiOsPrintf ("\n");
}

/* Free this external info block and move on to next external */
Expand Down
5 changes: 3 additions & 2 deletions source/compiler/aslload.c
Expand Up @@ -783,8 +783,9 @@ LdNamespace1Begin (
}
else
{
AslError (ASL_ERROR, ASL_MSG_SCOPE_TYPE, Op,
Op->Asl.ExternalName);
sprintf (MsgBuffer, "%s [%s]", Op->Asl.ExternalName,
AcpiUtGetTypeName (Node->Type));
AslError (ASL_ERROR, ASL_MSG_SCOPE_TYPE, Op, MsgBuffer);
return_ACPI_STATUS (AE_OK);
}
}
Expand Down
1 change: 1 addition & 0 deletions source/include/aclocal.h
Expand Up @@ -1357,6 +1357,7 @@ typedef struct acpi_external_list
#define ACPI_EXT_ORIGIN_FROM_FILE 0x02 /* External came from a file */
#define ACPI_EXT_INTERNAL_PATH_ALLOCATED 0x04 /* Deallocate internal path on completion */
#define ACPI_EXT_EXTERNAL_EMITTED 0x08 /* External() statement has been emitted */
#define ACPI_EXT_ORIGIN_FROM_OPCODE 0x10 /* External came from a External() opcode */


typedef struct acpi_external_file
Expand Down

0 comments on commit 16cd087

Please sign in to comment.