From c09798622bb725d93b9d0c09a9456481e906483e Mon Sep 17 00:00:00 2001 From: Achilleas Anagnostopoulos Date: Sat, 6 Jan 2018 10:35:26 +0000 Subject: [PATCH] acpi: provide more robust implementation for parsing AML method bodies The previous implementation used brute-force approach where the parser made an initial pass scanning the AML bytestream and looking for method declaration opcodes. It then parsed out the method name and arg count and populated a map which was used to detect the number of arguments to be parsed upon encountering a method invocation. This approach proved to be error-prone and would lead to an incorrect parse tree in the following ASL example: Method (FOOF, 1, NotSerialized) { Return ("bar") } Method (TST0, 0, NotSerialized) { FOOF(0) \_SB.FOOF(2, 3) } Scope(\_SB){ // Another FOOF method in \_SB which takes a different arg count Method (FOOF, 2, NotSerialized) { Return ("something") } } In the above example the parser would correctly parse the first FOOF call in TST0 but fail to parse the second invocation since the method name contains a scope. The second invocation would actually yield the following incorrect entity list (arguments appear as sibling entities): Ref(\_SB.FOOF), Const(2), Const(3) The new approach gets rid of the brute-force method and instead modifies the initial parse of the tree not to parse the entities in the AML method bodies but to instead track the start and end offset in the AML stream for the body contents. In the second pass (where the parser normally resolves symbol references), the parser can properly parse the contents of method bodies since the entire AML tree is now known and the parser can use the regular scope lookup rules to find the correct method declaration for the invocation and figure out the argument count it needs to parse. --- src/gopheros/device/acpi/aml/entity/entity.go | 29 ++-- .../device/acpi/aml/entity/entity_test.go | 10 +- .../device/acpi/aml/parser/opcode_table.go | 3 + src/gopheros/device/acpi/aml/parser/parser.go | 144 ++++++++---------- .../device/acpi/aml/parser/parser_test.go | 137 ++++++++--------- .../device/acpi/aml/parser/stream_reader.go | 3 + .../parser-testsuite-fwd-decls-DSDT.aml | Bin 62 -> 108 bytes .../parser-testsuite-fwd-decls-DSDT.dsl | 17 ++- 8 files changed, 155 insertions(+), 188 deletions(-) diff --git a/src/gopheros/device/acpi/aml/entity/entity.go b/src/gopheros/device/acpi/aml/entity/entity.go index 285382c..5e2b85e 100644 --- a/src/gopheros/device/acpi/aml/entity/entity.go +++ b/src/gopheros/device/acpi/aml/entity/entity.go @@ -1,6 +1,8 @@ package entity -import "gopheros/kernel" +import ( + "gopheros/kernel" +) // Entity is an interface implemented by all AML entities. type Entity interface { @@ -731,6 +733,9 @@ type Method struct { ArgCount uint8 Serialized bool SyncLevel uint8 + + BodyStartOffset uint32 + BodyEndOffset uint32 } // NewMethod creats a new AML method entity. @@ -775,37 +780,21 @@ func (ent *Method) SetArg(argIndex uint8, arg interface{}) bool { type Invocation struct { Generic - MethodName string - MethodDef *Method + Method *Method } // NewInvocation creates a new method invocation object. -func NewInvocation(tableHandle uint8, name string, args []interface{}) *Invocation { +func NewInvocation(tableHandle uint8, method *Method, args []interface{}) *Invocation { return &Invocation{ Generic: Generic{ op: OpMethodInvocation, tableHandle: tableHandle, args: args, }, - MethodName: name, + Method: method, } } -// ResolveSymbolRefs receives as input the root of the AML entity tree and -// attempts to resolve any symbol references using the scope searching rules -// defined by the ACPI spec. -func (ent *Invocation) ResolveSymbolRefs(rootNS Container) *kernel.Error { - var ok bool - if ent.MethodDef, ok = FindInScope(ent.Parent(), rootNS, ent.MethodName).(*Method); !ok { - return &kernel.Error{ - Module: "acpi_aml_vm", - Message: "could not resolve method invocation to: " + ent.MethodName + "; parent: " + ent.Parent().Name(), - } - } - - return nil -} - // Device defines an AML device entity. type Device struct { Scope diff --git a/src/gopheros/device/acpi/aml/entity/entity_test.go b/src/gopheros/device/acpi/aml/entity/entity_test.go index a4efcd7..9df2eed 100644 --- a/src/gopheros/device/acpi/aml/entity/entity_test.go +++ b/src/gopheros/device/acpi/aml/entity/entity_test.go @@ -25,7 +25,7 @@ func TestEntityMethods(t *testing.T) { {NewBankField(42), OpBankField, ""}, {NewReference(42, "TRG0"), OpName, ""}, {NewMethod(42, "FOO0"), OpMethod, "FOO0"}, - {NewInvocation(42, "MTH0", nil), OpMethodInvocation, ""}, + {NewInvocation(42, NewMethod(42, "MTH0"), nil), OpMethodInvocation, ""}, {NewMutex(42), OpMutex, ""}, {NewDevice(42, "DEV0"), OpDevice, "DEV0"}, {NewProcessor(42, "CPU0"), OpProcessor, "CPU0"}, @@ -283,14 +283,6 @@ func TestLazySymbolResolver(t *testing.T) { &Reference{TargetName: "FLD0"}, "", }, - { - &Invocation{MethodName: "UNKNOWN"}, - `could not resolve method invocation to: UNKNOWN; parent: \`, - }, - { - &Invocation{MethodName: "MTH0"}, - "", - }, } for specIndex, spec := range specs { diff --git a/src/gopheros/device/acpi/aml/parser/opcode_table.go b/src/gopheros/device/acpi/aml/parser/opcode_table.go index 404592f..e1333d8 100644 --- a/src/gopheros/device/acpi/aml/parser/opcode_table.go +++ b/src/gopheros/device/acpi/aml/parser/opcode_table.go @@ -142,6 +142,9 @@ type opcodeInfo struct { argFlags opArgFlags } +// Used by Parser.parseMethodBody for deferred parsing of method bodies. +const methodOpInfoIndex = 0x0d + // The opcode table contains all opcode-related information that the parser knows. // This table is modeled after a similar table used in the acpica implementation. var opcodeTable = []opcodeInfo{ diff --git a/src/gopheros/device/acpi/aml/parser/parser.go b/src/gopheros/device/acpi/aml/parser/parser.go index 7daa950..8f4d495 100644 --- a/src/gopheros/device/acpi/aml/parser/parser.go +++ b/src/gopheros/device/acpi/aml/parser/parser.go @@ -14,6 +14,13 @@ var ( errResolvingEntities = &kernel.Error{Module: "acpi_aml_parser", Message: "AML bytecode contains unresolvable entities"} ) +type parseOpt uint8 + +const ( + parseOptSkipMethodBodies parseOpt = iota + parseOptParseMethodBodies +) + // Parser implements an AML parser. type Parser struct { r amlStreamReader @@ -23,19 +30,14 @@ type Parser struct { tableName string tableHandle uint8 - // methodArgCount is initialized in a pre-parse step with the names and expected - // number of args for each function declaration. This is required as function - // invocations do not employ any mechanism to indicate the number of args that - // need to be parsed. Moreover, the spec allows for forward function declarations. - methodArgCount map[string]uint8 + parseOptions parseOpt } // NewParser returns a new AML parser instance. func NewParser(errWriter io.Writer, rootEntity entity.Container) *Parser { return &Parser{ - errWriter: errWriter, - root: rootEntity, - methodArgCount: make(map[string]uint8), + errWriter: errWriter, + root: rootEntity, } } @@ -51,12 +53,9 @@ func (p *Parser) ParseAML(tableHandle uint8, tableName string, header *table.SDT uint32(unsafe.Sizeof(table.SDTHeader{})), ) - // Pass 1: scan bytecode and locate all method declarations. This allows us to - // properly parse the arguments to method invocations at pass 2 even if the - // the name of the invoked method is a forward reference. - p.detectMethodDeclarations() - - // Pass 2: decode bytecode and build entitites + // Pass 1: decode bytecode and build entitites without recursing into + // function bodies. + p.parseOptions = parseOptSkipMethodBodies p.scopeStack = nil p.scopeEnter(p.root) if !p.parseObjList(header.Length) { @@ -66,11 +65,15 @@ func (p *Parser) ParseAML(tableHandle uint8, tableName string, header *table.SDT } p.scopeExit() - // Pass 3: check parents and resolve symbol references + // Pass 2: parse method bodies, check entity parents and resolve all + // symbol references var resolveFailed bool entity.Visit(0, p.root, entity.TypeAny, func(_ int, ent entity.Entity) bool { - // Skip method bodies; their contents will be lazily resolved by the interpreter - if _, isMethod := ent.(*entity.Method); isMethod { + if method, isMethod := ent.(*entity.Method); isMethod { + resolveFailed = resolveFailed || !p.parseMethodBody(method) + + // Don't recurse into method bodies; their contents + // will be lazilly resolved by the VM return false } @@ -101,57 +104,6 @@ func (p *Parser) ParseAML(tableHandle uint8, tableName string, header *table.SDT return nil } -// detectMethodDeclarations scans the AML byte-stream looking for function -// declarations. For each discovered function, the method will parse its flags -// and update the methodArgCount map with the number of required arguments. -func (p *Parser) detectMethodDeclarations() { - var ( - next *opcodeInfo - method string - startOffset = p.r.Offset() - curOffset, pkgLen uint32 - flags uint64 - ok bool - ) - - for !p.r.EOF() { - if next, ok = p.nextOpcode(); !ok { - // Skip one byte to the right and try again. Maybe we are stuck inside - // the contents of a string or buffer - _, _ = p.r.ReadByte() - continue - } - - if next.op != entity.OpMethod { - continue - } - - // Parse pkg len; if this fails then this is not a method declaration - curOffset = p.r.Offset() - if pkgLen, ok = p.parsePkgLength(); !ok { - continue - } - - // Parse method name - if method, ok = p.parseNameString(); !ok { - continue - } - - // The next byte encodes the method flags which also contains the arg count - // at bits 0:2 - if flags, ok = p.parseNumConstant(1); !ok { - continue - } - - p.methodArgCount[method] = uint8(flags) & 0x7 - - // At this point we can use the pkg length to skip over the term list - p.r.SetOffset(curOffset + pkgLen) - } - - p.r.SetOffset(startOffset) -} - // parseObjList tries to parse an AML object list. Object lists are usually // specified together with a pkgLen block which is used to calculate the max // read offset that the parser may reach. @@ -174,8 +126,7 @@ func (p *Parser) parseObj() bool { ) // If we cannot decode the next opcode then this may be a method - // invocation or a name reference. If neither is the case, we need to - // rewind the stream and parse a method invocation before giving up. + // invocation or a name reference. curOffset = p.r.Offset() if info, ok = p.nextOpcode(); !ok { p.r.SetOffset(curOffset) @@ -352,6 +303,16 @@ func (p *Parser) parseArg(info *opcodeInfo, obj entity.Entity, argIndex uint8, a case opArgTarget: arg, ok = p.parseTarget() case opArgTermList: + // If this is a method and the SkipMethodBodies option is set + // then record the body start and end offset so we can parse + // it at a later stage. + if method, isMethod := obj.(*entity.Method); isMethod && p.parseOptions == parseOptSkipMethodBodies { + method.BodyStartOffset = p.r.Offset() + method.BodyEndOffset = maxReadOffset + p.r.SetOffset(maxReadOffset) + return true + } + // If object is a scoped entity enter it's scope before parsing // the term list. Otherwise, create an unnamed scope, attach it // as the next argument to obj and enter that. @@ -450,6 +411,25 @@ func (p *Parser) makeObjForOpcode(info *opcodeInfo) entity.Entity { return obj } +// parseMethodBody parses the entities that make up a method's body. After the +// entire AML tree has been parsed, the parser makes a second pass and calls +// parseMethodBody for each Method entity. +// +// By deferring the parsing of the method body, we ensure that the parser can +// lookup the method declarations (even if forward declarations are used) for +// each method invocation. As method declarations contain information about the +// expected argument count, the parser can use this information to properly +// parse the invocation arguments. For more details see: parseNamedRef +func (p *Parser) parseMethodBody(method *entity.Method) bool { + p.parseOptions = parseOptParseMethodBodies + p.scopeEnter(method) + p.r.SetOffset(method.BodyStartOffset) + ok := p.parseArg(&opcodeTable[methodOpInfoIndex], method, 2, opArgTermList, method.BodyEndOffset) + p.scopeExit() + + return ok +} + // parseNamedRef attempts to parse either a method invocation or a named // reference. As AML allows for forward references, the actual contents for // this entity will not be known until the entire AML stream has been parsed. @@ -464,15 +444,17 @@ func (p *Parser) parseNamedRef() bool { return false } - var ( - curOffset uint32 - argIndex uint8 - arg entity.Entity - argList []interface{} - ) + // Check if this is a method invocation + ent := entity.FindInScope(p.scopeCurrent(), p.root, name) + if methodDef, isMethod := ent.(*entity.Method); isMethod { + var ( + curOffset uint32 + argIndex uint8 + arg entity.Entity + argList []interface{} + ) - if argCount, isMethod := p.methodArgCount[name]; isMethod { - for argIndex < argCount && !p.r.EOF() { + for argIndex < methodDef.ArgCount && !p.r.EOF() { // Peek next opcode curOffset = p.r.Offset() nextOpcode, ok := p.nextOpcode() @@ -501,12 +483,12 @@ func (p *Parser) parseNamedRef() bool { } // Check whether all expected arguments have been parsed - if argIndex != argCount { - kfmt.Fprintf(p.errWriter, "[table: %s, offset: %d] unexpected arglist end for method %s invocation: expected %d; got %d\n", p.tableName, p.r.Offset(), name, argCount, argIndex) + if argIndex != methodDef.ArgCount { + kfmt.Fprintf(p.errWriter, "[table: %s, offset: %d] unexpected arglist end for method %s invocation: expected %d; got %d\n", p.tableName, p.r.Offset(), name, methodDef.ArgCount, argIndex) return false } - return p.scopeCurrent().Append(entity.NewInvocation(p.tableHandle, name, argList)) + return p.scopeCurrent().Append(entity.NewInvocation(p.tableHandle, methodDef, argList)) } // Otherwise this is a reference to a named entity diff --git a/src/gopheros/device/acpi/aml/parser/parser_test.go b/src/gopheros/device/acpi/aml/parser/parser_test.go index fa223a1..78d3bbf 100644 --- a/src/gopheros/device/acpi/aml/parser/parser_test.go +++ b/src/gopheros/device/acpi/aml/parser/parser_test.go @@ -36,6 +36,63 @@ func TestParser(t *testing.T) { } } +func TestParsingOfMethodBodies(t *testing.T) { + var resolver = mockResolver{ + tableFiles: []string{"parser-testsuite-fwd-decls-DSDT.aml"}, + } + + p := NewParser(os.Stderr, genDefaultScopes()) + tableName := strings.Replace(resolver.tableFiles[0], ".aml", "", -1) + if err := p.ParseAML(0, tableName, resolver.LookupTable(tableName)); err != nil { + t.Fatalf("[%s]: %v", tableName, err) + } + + // Collect invocations + var invocations []*entity.Invocation + entity.Visit(0, p.root, entity.TypeAny, func(_ int, ent entity.Entity) bool { + if inv, isInv := ent.(*entity.Invocation); isInv { + invocations = append(invocations, inv) + } + return true + }) + + specs := []struct { + expParentName string + expArgCount int + }{ + // Call to `\NST1` + {`\`, 1}, + // Call to `\_SB.NST1` + {`_SB_`, 2}, + // Call to `\NST1` (first argument of above invocation) + {`\`, 1}, + } + + if exp, got := len(specs), len(invocations); exp != got { + t.Fatalf("expected parser to produce %d method invocations; got %d", exp, got) + } + + for specIndex, spec := range specs { + if got := invocations[specIndex].Method.Parent().Name(); got != spec.expParentName { + t.Errorf( + "[spec %d] expected invocation to target %s.%s; got %s.%s", + specIndex, + spec.expParentName, invocations[specIndex].Method.Name(), + got, invocations[specIndex].Method.Name(), + ) + } + + if got := len(invocations[specIndex].Args()); got != spec.expArgCount { + t.Errorf( + "[spec %d] expected invocation to target %s.%s to receive %d args; got %d", + specIndex, + spec.expParentName, invocations[specIndex].Method.Name(), spec.expArgCount, + got, + ) + } + } +} + func TestTableHandleAssignment(t *testing.T) { var resolver = mockResolver{tableFiles: []string{"parser-testsuite-DSDT.aml"}} @@ -326,11 +383,12 @@ func TestParserErrorHandling(t *testing.T) { }) t.Run("parseNamedRef errors", func(t *testing.T) { - t.Run("missing args", func(t *testing.T) { + t.Run("incorrect args for method", func(t *testing.T) { p.root = entity.NewScope(entity.OpScope, 42, `\`) - p.methodArgCount = map[string]uint8{ - "MTHD": 10, - } + + methodDecl := entity.NewMethod(42, "MTHD") + methodDecl.ArgCount = 5 + p.root.Append(methodDecl) mockParserPayload(p, []byte{ 'M', 'T', 'H', 'D', @@ -605,77 +663,6 @@ func TestParserErrorHandling(t *testing.T) { }) } -func TestDetectMethodDeclarations(t *testing.T) { - p := &Parser{ - errWriter: ioutil.Discard, - } - - validMethod := []byte{ - byte(entity.OpMethod), - 5, // pkgLen - 'M', 'T', 'H', 'D', - 2, // flags (2 args) - } - - t.Run("success", func(t *testing.T) { - mockParserPayload(p, validMethod) - p.methodArgCount = make(map[string]uint8) - p.detectMethodDeclarations() - - argCount, inMap := p.methodArgCount["MTHD"] - if !inMap { - t.Error(`detectMethodDeclarations failed to parse method "MTHD"`) - } - - if exp := uint8(2); argCount != exp { - t.Errorf(`expected arg count for "MTHD" to be %d; got %d`, exp, argCount) - } - }) - - t.Run("bad pkgLen", func(t *testing.T) { - mockParserPayload(p, []byte{ - byte(entity.OpMethod), - // lead byte bits (6:7) indicate 1 extra byte that is missing - byte(1 << 6), - }) - - p.methodArgCount = make(map[string]uint8) - p.detectMethodDeclarations() - }) - - t.Run("error parsing namestring", func(t *testing.T) { - mockParserPayload(p, append([]byte{ - byte(entity.OpMethod), - byte(5), // pkgLen - 10, // bogus char, not part of namestring - }, validMethod...)) - - p.methodArgCount = make(map[string]uint8) - p.detectMethodDeclarations() - - argCount, inMap := p.methodArgCount["MTHD"] - if !inMap { - t.Error(`detectMethodDeclarations failed to parse method "MTHD"`) - } - - if exp := uint8(2); argCount != exp { - t.Errorf(`expected arg count for "MTHD" to be %d; got %d`, exp, argCount) - } - }) - - t.Run("error parsing method flags", func(t *testing.T) { - mockParserPayload(p, []byte{ - byte(entity.OpMethod), - byte(5), // pkgLen - 'F', 'O', 'O', 'F', - // Missing flag byte - }) - - p.methodArgCount = make(map[string]uint8) - p.detectMethodDeclarations() - }) -} - func mockParserPayload(p *Parser, payload []byte) *table.SDTHeader { resolver := fixedPayloadResolver{payload} header := resolver.LookupTable("DSDT") diff --git a/src/gopheros/device/acpi/aml/parser/stream_reader.go b/src/gopheros/device/acpi/aml/parser/stream_reader.go index 4b29d67..fc151e3 100644 --- a/src/gopheros/device/acpi/aml/parser/stream_reader.go +++ b/src/gopheros/device/acpi/aml/parser/stream_reader.go @@ -80,5 +80,8 @@ func (r *amlStreamReader) Offset() uint32 { // SetOffset sets the reader offset to the supplied value. func (r *amlStreamReader) SetOffset(off uint32) { + if max := uint32(len(r.data)); off > max { + off = max + } r.offset = off } diff --git a/src/gopheros/device/acpi/table/tabletest/parser-testsuite-fwd-decls-DSDT.aml b/src/gopheros/device/acpi/table/tabletest/parser-testsuite-fwd-decls-DSDT.aml index 00d0f4d23bb18bdcc8ed07d53ee7c2129ff9bb4f..68fbecc66021e36dc20ff68187819d850a13a963 100644 GIT binary patch delta 89 zcmcD^;c^Lf3CUq#U|{+)kxR-$Kq5ZaDPBa#FF3@IX$fy}er{?>MrK|*gNPzf$bb