[llvm-dev] X86 Disassembler Misplaced Assignment

[Craig Topper via llvm-dev]

Based on git blame, it looks like its been like since the file was created
in 2009. Based on that we should probably just keep the one assignment
above the switch.
~Craig


On Mon, Dec 16, 2019 at 1:31 PM Smith, Cameron T via llvm-dev <
llvm-dev at lists.llvm.org> wrote:

> In the file llvm/lib/Target/X86/Disassembler/X86DisassemblerDecoder.cpp in
> the function readDisplacement (line 1235),insn->consumedDisplacement is
> set to true on line 1245. In the proceeding switch statement, the same
> variable may be set to false, but the line immediately after the switch
> (line 1269) always sets it back to true. Here’s a copy of the source:
>
>
>
> */**
>
> ** readDisplacement - Consumes the displacement of an instruction.*
>
> ***
>
> ** @param insn  - The instruction whose displacement is to be read.*
>
> ** @return      - 0 if the displacement byte was successfully read;
> nonzero*
>
> **                otherwise.*
>
> **/*
>
> static int readDisplacement(struct InternalInstruction* insn) {
>
>   int8_t d8;
>
>   int16_t d16;
>
>   int32_t d32;
>
>
>
>   dbgprintf(insn, "readDisplacement()");
>
>
>
>   *if* (insn->consumedDisplacement)
>
>     *return* 0;
>
>
>
>   insn->consumedDisplacement = true;     // The value is always set to
> ‘true’ here
>
>   insn->displacementOffset = insn->readerCursor - insn->startLocation;
>
>
>
>   *switch* (insn->eaDisplacement) {
>
>   *case* EA_DISP_NONE:
>
>     insn->consumedDisplacement = false;  // The value may be set to
> ‘false’ here
>
>     *break*;                               // Control flow skips to the
> end of the switch
>
>   *case* EA_DISP_8:
>
>     *if* (consumeInt8(insn, &d8))
>
>       *return* -1;
>
>     insn->displacement = d8;
>
>     *break*;
>
>   *case* EA_DISP_16:
>
>     *if* (consumeInt16(insn, &d16))
>
>       *return* -1;
>
>     insn->displacement = d16;
>
>     *break*;
>
>   *case* EA_DISP_32:
>
>     *if* (consumeInt32(insn, &d32))
>
>       *return* -1;
>
>     insn->displacement = d32;
>
>     *break*;
>
>   }
>
>
>
>   insn->consumedDisplacement = true;     // The value is always set to
> ‘true’ again, reversing the change that was
>
>                                          // made if insn->eaDisplacement
> == EA_DISP_NONE
>
>   *return* 0;
>
> }
>
>
>
> I’m not sure if this is a bug or if it was intentional. If the logic is
> not correct, what would be the appropriate fix? If it would be more
> appropriate to file a formal bug report let me know and I can file one.
>
>
>
> ~ Cameron Smith
>
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20191216/da05458a/attachment.html>