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

GEN_ABSOLUTE_SYM cannot handle value larger than INT_MAX on qemu_x86_64 #22542

Closed
dcpleung opened this issue Feb 6, 2020 · 7 comments · Fixed by #22764
Closed

GEN_ABSOLUTE_SYM cannot handle value larger than INT_MAX on qemu_x86_64 #22542

dcpleung opened this issue Feb 6, 2020 · 7 comments · Fixed by #22764
Assignees
Labels
area: X86_64 x86-64 Architecture (64-bit) bug The issue is a bug, or the PR is fixing a bug priority: low Low impact/importance bug

Comments

@dcpleung
Copy link
Member

dcpleung commented Feb 6, 2020

If a kconfig (e.g. CONFIG_SYS_CLOCK_HW_CYCLES_PER_SEC is defined with a value > +2147483647, it fails to compile with following error:

In file included from ../include/toolchain.h:39,
                 from zephyr/misc/generated/configs.c:7:
zephyr/misc/generated/configs.c: In function '_ConfigAbsSyms':
../include/toolchain/gcc.h:404:2: error: invalid 'asm': operand is not a condition code, invalid operand code 'c'
  404 |  __asm__(".globl\t" #name "\n\t.equ\t" #name \
      |  ^~~~~~~
zephyr/misc/generated/configs.c:14:1: note: in expansion of macro 'GEN_ABSOLUTE_SYM'
   14 | GEN_ABSOLUTE_SYM(CONFIG_SYS_CLOCK_HW_CYCLES_PER_SEC, 2147483648);
      | ^~~~~~~~~~~~~~~~

I haven't tried negative maximum but it is reasonable to believe that it would also fail to compile.

@dcpleung dcpleung added bug The issue is a bug, or the PR is fixing a bug area: X86_64 x86-64 Architecture (64-bit) labels Feb 6, 2020
@jhedberg jhedberg added the priority: low Low impact/importance bug label Feb 11, 2020
@andrewboie
Copy link
Contributor

I'll take a look.

@andrewboie
Copy link
Contributor

The "c" modifier:

c | Require a constant operand and print the constant expression with no punctuation.
-- | --

From https://gcc.gnu.org/onlinedocs/gcc/Extended-Asm.html in 6.47.2.8 x86 Operand Modifiers

Seems like it is not a fan of constants that don't fit in a 32-bit integer? hmmm

@dcpleung
Copy link
Member Author

Note that this fails only on qemu_x86_64, but works on qemu_x86.

@andrewboie
Copy link
Contributor

This boils down to the following GCC inline asm code:

__asm__(".globl CONFIG_SYS_CLOCK_HW_CYCLES_PER_SEC\n\t"
        ".equ CONFIG_SYS_CLOCK_HW_CYCLES_PER_SEC,%c0\n\t"
        ".type CONFIG_SYS_CLOCK_HW_CYCLES_PER_SEC,@object"
		: : "n"(3000000000));

We are trying to set the value with .equ but it is not happy with the combination of %c0 for the first operand, modified with c, and the operand constraint is itself n.

The fact that this works fine with x86 32-bit is truly baffling.

@andrewboie
Copy link
Contributor

I might have to ask in the GCC mailing list about this one. The documentation for all of this is so far not helping.

@andrewboie
Copy link
Contributor

andrewboie commented Feb 11, 2020

Note that this fails only on qemu_x86_64, but works on qemu_x86.

It passes if the value is less than UINT_MAX, if it's bigger than that it still builds but the value is truncated

@andrewboie
Copy link
Contributor

So what I'm seeing here is as follows:

  • This mechanism likely does not work for any arch if the value exceeds UINT_MAX. In addition, on i386 it seems to fail silently. We might need some other macro type for values that can get this large.
  • This mechanism causes an error on x86-64 if the value exceeds INT_MAX, unlike other arches.

For the former, we probably should open another bug if we're expecting to start dealing with Kconfigs that have values larger than UINT_MAX. We might consider other alternatives such as having CONFIG_SYS_CLOCK_HW_CYCLES_PER_SEC be expressed in terms of KHz instead of Hz, but that may or may not be feasible.

For the latter, I've sent a message to gcc-help, maybe someone knows a workaround.

andrewboie pushed a commit to andrewboie/zephyr that referenced this issue Feb 12, 2020
Use the special x86 operand modifier 'p' to print the raw value.
This fixes an issue on x86-64 where errors were generated
if a constant larger than INT_MAX was used.

Values larger than UINT_MAX are still unsupported (on any arch).

Fixes: zephyrproject-rtos#22542

Signed-off-by: Andrew Boie <andrew.p.boie@intel.com>
jhedberg pushed a commit that referenced this issue Feb 13, 2020
Use the special x86 operand modifier 'p' to print the raw value.
This fixes an issue on x86-64 where errors were generated
if a constant larger than INT_MAX was used.

Values larger than UINT_MAX are still unsupported (on any arch).

Fixes: #22542

Signed-off-by: Andrew Boie <andrew.p.boie@intel.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: X86_64 x86-64 Architecture (64-bit) bug The issue is a bug, or the PR is fixing a bug priority: low Low impact/importance bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants