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

Go Integer overflows translated from large C enum numbers #133

Open
hxy9243 opened this issue Oct 30, 2022 · 5 comments
Open

Go Integer overflows translated from large C enum numbers #133

hxy9243 opened this issue Oct 30, 2022 · 5 comments

Comments

@hxy9243
Copy link

hxy9243 commented Oct 30, 2022

In C, some legit enum definitions can generate problematic generations in Go. For example:

typedef enum
{
    STATUS_INIT = 0,
    STATUS_STARTED = 1,
    STATUS_RUNNING = 2,
    ...
    STATUS_FAILED = 0xFFFFFFFF,
} example_status_t;

The STATUS_FAILED can be translated to unsigned integer in Go, which for a int32, STATUS_FAILED should really be a -1.

The generated code with configuration TRANSLATOR.ConstRules.enum: eval.

// Status_t as declared in large-const/large-const.h:17
type Status_t int32

// Status_t enumeration from large-const/large-const.h:17
const (
        ...
	STATUS_FAILED   Status_t = 4294967295
)

With the rule expand, it'll generate a similar problem:

	STATUS_FAILED   Status_t = C.STATUS_FAILED

which throws Go compile error: cannot use (_Ciconst_STATUS_FAILED) (untyped int constant 4294967295) as Status_t value in constant declaration (overflows).

A viable solution might be:

  • Generate uint32 for C enums.
  • Eval the C evals as signed integers and interpreter them with signs.

This can happen in many libraries that uses large constant numbers like 0x10000000 or 0xFFFF0000.

@lotodore
Copy link
Contributor

lotodore commented Nov 2, 2022

I understand the problem. At the same time, the statement

In C, some legit enum definitions

is kind of questionable, I'm afraid. According to C99, enum is supposed to be represented by an integer, however, it is not specified which actual type or signedness is to be used. The meaning of an enum value STATUS_FAILED = 0xFFFFFFFF therefore depends on the compiler implementation. This means it is not as "legit" as it may seem.

That being said, it would probably make sense to use e.g. gcc as reference. But this is not a trivial problem. As enums can be evaluated, they can overflow during evaluation, and if the expression is evaluated in Go (because it is not evaluated before translation), the enum value may have a different result.

@xlab
Copy link
Owner

xlab commented Nov 2, 2022

google/flatbuffers#5161

not so easy even in C/C++ codebases

@hxy9243
Copy link
Author

hxy9243 commented Nov 2, 2022

https://github.com/RadeonOpenCompute/rocm_smi_lib/blob/master/include/rocm_smi/rocm_smi.h#L130

Sometimes people do use this pattern in enums.

There are workarounds for it. So if it's too much effort for a corner case, I'm all for closing this ticket.

@xlab
Copy link
Owner

xlab commented Nov 2, 2022

@hxy9243 definitely not closing, coz we may eventually find a workaround. The target of c-for-go is to be as practical as possible

@hxy9243
Copy link
Author

hxy9243 commented Nov 2, 2022

Sure. Right now I'm using hard-coded bash and sed to translate certain numbers in C header files. Would love a more elegant solution with c-for-go.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants