Skip to content

[SYCL]Fix objdump's bug on macho arm64 when adrp's imm < 0 #2499

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

Closed

Conversation

JuunChen
Copy link

@JuunChen JuunChen commented Sep 20, 2020

Problem

image

I used the same llvm-objdump -S -m --section=<section name> <macho-file> command for two different macho files to see the disassembly code.
But the annotation for the assembly instruction can't show for the second file, as shown on the right.

Here is the two files which you can test:
the two macho files.zip

Reason

I found this to be a bug if the adrp's imm < 0, at the followed lines:

 adrp_imm = ((info->adrp_inst & 0x00ffffe0) >> 3) | ((info->adrp_inst >> 29) & 0x3);
 if (info->adrp_inst & 0x0200000)
      adrp_imm |= 0xfffffffffc000000LL;

The line adrp_imm = ((info->adrp_inst & 0x00ffffe0) >> 3) | ((info->adrp_inst >> 29) & 0x3) find the adrp_imm in adrp_inst, it's right.
And the next two lines are intended to:

  1. Determine if adrp_imm is a negative number
  2. if the adrp_imm is negative , adrp_imms's 64-bit complement is calculated

However, as shown in the picture:
image
adrp_imm is encoded as [23:5][31:29].

If you want to determine if adrp_imm is a negative number, you should determine the 23rd bit of adrp_inst, or the 20th bit of adrp_imm.
It will not be info->adrp_inst & 0x0200000. The 0x0200000 is 0b00000000001000000000000000000000,the code is to determine the 21st of adrp_inst,so it doesn't make any sense.
This code adrp_imm |= 0xfffffffffc000000LL is also wrong, it caculated the wrong bits.

Fix

Use the follow code to fix:

if (adrp_imm & (1 << (21 - 1)))
       adrp_imm |= ~((1LL << 21) - 1);

validation

After the correction, I got the result I wanted.

@JuunChen JuunChen requested a review from bader as a code owner September 20, 2020 08:41
chenjun.jonas added 3 commits September 20, 2020 17:50
## Problem
![image](https://user-images.githubusercontent.com/13558319/93706082-701d9400-fb55-11ea-8d34-d72a9f75ec48.png)

I used the same `llvm-objdump -S -m --section=<section name> <macho-file>` command for two different macho files to see the disassembly code.
But the annotation for the assembly instruction can't show for the second file,  as shown on the right.

Here is the two files which you can test:
[the two macho files.zip](https://github.com/intel/llvm/files/5251278/the.two.macho.files.zip)

## Reason
I found this to be a bug if the adrp's imm < 0,  at the followed lines:

```
 adrp_imm = ((info->adrp_inst & 0x00ffffe0) >> 3) | ((info->adrp_inst >> 29) & 0x3);
 if (info->adrp_inst & 0x0200000)
      adrp_imm |= 0xfffffffffc000000LL;
```

The line adrp_imm = ((info->adrp_inst & 0x00ffffe0) >> 3) | ((info->adrp_inst >> 29) & 0x3) find the adrp_imm in adrp_inst, it's right.
And the next two lines are intended to:
1. Determine if adrp_imm is a negative number
2. if the adrp_imm is negative , adrp_imms's 64-bit complement is calculated

However, as shown in the picture:
![image](https://user-images.githubusercontent.com/13558319/93705478-52016500-fb50-11ea-8cc1-6b82c8bdeb17.png)
adrp_imm is encoded as [23:5][31:29].

If you want to determine if adrp_imm is a negative number, you should determine the 23rd bit of adrp_inst, or the 20th bit of adrp_imm.
It will not be info->adrp_inst & 0x0200000. The 0x0200000 is 0b00000000001000000000000000000000,the code is to determine the 21st of adrp_inst,so it doesn't make any sense.
This code `adrp_imm |= 0xfffffffffc000000LL` is also wrong, it caculated the wrong bits.

## Fix
Use the follow code to fix:
```
if (adrp_imm & (1 << (21 - 1)))
       adrp_imm |= ~((1LL << 21) - 1);
```

## validation
After the correction, I got the result I wanted.

Differential Revision: https://reviews.llvm.org/D87985
@JuunChen JuunChen closed this Sep 20, 2020
jsji pushed a commit that referenced this pull request Apr 18, 2024
kbenzie pushed a commit to kbenzie/intel-llvm that referenced this pull request Feb 17, 2025
Chenyang-L pushed a commit that referenced this pull request Feb 18, 2025
[CTS] adjust timestamp test
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.

1 participant