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

Bitfield not rejected in type decl #1032

Closed
Tracked by #1481
bbannier opened this issue Oct 29, 2021 · 4 comments
Closed
Tracked by #1481

Bitfield not rejected in type decl #1032

bbannier opened this issue Oct 29, 2021 · 4 comments
Assignees

Comments

@bbannier
Copy link
Member

bbannier commented Oct 29, 2021

Currently the following code passes validation, but fails with an internal error when compiled:

# file: /tmp/bitfield.spicy
module foo;

type X = bitfield(32) {
    x: 0..4;
};
$ spicyc -j /tmp/bitfield.spicy
|| - spicy::type::Bitfield (bitset.spicy:3:10-5:2) (non-const) (type-id: foo::X) (resolved) [@s:7f81cb412c60]
||  - type::UnsignedInteger <width="32"> (bitset.spicy:3:10-5:2) (const) (resolved) [@t:7f81cb411e70]
||  - type::Tuple <wildcard="false"> (bitset.spicy:3:10-5:2) (const) (resolved) [@t:7f81cb48ee30]
||   - type::tuple::Element (bitset.spicy:3:10-5:2) [@t:7f81cb48f120]
||    - ID <name="x"> (bitset.spicy:4:5) [@i:7f81cb48e010]
||    - type::UnsignedInteger <width="32"> (const) (resolved) [@t:7f81cb4120e0]
||  - spicy::type::bitfield::Bits <field_width="32" lower="0" upper="4"> (bitset.spicy:4:5) [@s:7f81cb412c00]
||   - ID <name="x"> (bitset.spicy:4:5) [@i:7f81cb412240]
||   - declaration::Expression %4 <linkage="private"> (bitset.spicy:4:5) [canon-id: foo::X::__dd] [@d:7f81cb412580]
||    - ID <name="__dd"> [@i:7f81cb4121d0]
||    - expression::Keyword <kind="$$"> (non-const) (resolved) [@e:7f81cb412170]
||     - type::UnsignedInteger <width="32"> (const) (resolved) [@t:7f81cb4120e0]
||    - node::None (bitset.spicy:4:5) [@n:7f81cb405fb0]
||   - type::UnsignedInteger <width="32"> (const) (resolved) [@t:7f81cb4120e0]
||   - node::None (bitset.spicy:4:5) [@n:7f81cb405fb0]
|| [internal-error] /private/tmp/bitset.spicy:3:10-5:2: codegen: type foo::X does not have a visitor
|| 
|| --- Aborting
|| # 0   libhilti.dylib                      0x000000010fef0696 _ZN5hilti2rt9BacktraceC1Ev + 118
|| # 1   libhilti.dylib                      0x000000010f87576b _ZN5hilti4util20abort_with_backtraceEv + 139
|| # 2   libhilti.dylib                      0x000000010f86e6f7 _ZN5hilti6Logger13internalErrorERKNSt3__112basic_stringIcNS1_11char_traitsIcEENS1_9allocatorIcEEEERKNS_8LocationE + 55
|| # 3   libhilti.dylib                      0x000000010f8fdb8a _ZN5hilti6Logger13internalErrorINS_4TypeELPv0EEEvNSt3__112basic_stringIcNS4_11char_traitsIcEENS4_9allocatorIcEEEERKT_ + 90
|| # 4   libhilti.dylib                      0x000000010f8f97d4 _ZN5hilti6detail7CodeGen7compileERKNS_4TypeENS0_7codegen9TypeUsageE + 2244
|| # 5   libhilti.dylib                      0x000000010f895ef5 _ZN5hilti6detail7visitor7VisitorIvN12_GLOBAL__N_114GlobalsVisitorENS_4NodeELNS1_5OrderE0EE8dispatchERKS5_ + 4053
|| # 6   libhilti.dylib                      0x000000010f87f10b _ZN12_GLOBAL__N_114GlobalsVisitor15addDeclarationsEPN5hilti6detail7CodeGenERKNS1_4NodeERKNS1_2IDEPNS2_3cxx4UnitEb + 219
|| # 7   libhilti.dylib                      0x000000010f87c26b _ZN5hilti6detail7CodeGen13compileModuleERNS_4NodeEPNS_4UnitEb + 891
|| # 8   libhilti.dylib                      0x000000010fa50418 _ZN5hilti4Unit7codegenEv + 744
|| # 9   libhilti.dylib                      0x000000010f9f862f _ZN5hilti6Driver13_codegenUnitsEv + 959
|| # 10  libhilti.dylib                      0x000000010f9f96f7 _ZN5hilti6Driver12compileUnitsEv + 1447
|| # 11  libhilti.dylib                      0x000000010f9fa2bd _ZN5hilti6Driver7compileEv + 45
|| # 12  libhilti.dylib                      0x000000010f9f9df9 _ZN5hilti6Driver3runEv + 185
|| # 13  spicyc                              0x000000010efa0467 main + 311
|| # 14  libdyld.dylib                       0x00007fff72201cc9 start + 1
|| 

We should probably reject using a type decl with a bitfield during validation, or alternatively add proper support for it.

bbannier added a commit that referenced this issue Oct 29, 2021
Since this is currently not supported without the validation users would
see compiler errors.

Closes #1032.
@bbannier bbannier self-assigned this Oct 29, 2021
@bbannier
Copy link
Member Author

bbannier commented Nov 1, 2021

Instead of rejecting bitfields here, we should add proper support instead, see the first sketch here.

@bbannier bbannier removed their assignment Nov 1, 2021
@bbannier
Copy link
Member Author

As a workaround, one can put the bitfield into a unit and extract the bitfield data after parsing; this allows reusing a single bitfield declaration.

type BF = unit {
	x: bitfield(8) {
		x1: 0..7;
	};
} &convert=self.x;

public type X = unit {
	x: BF;
};

@zeek-bot
Copy link

This issue has been mentioned on Zeek. There might be relevant details there:

https://community.zeek.org/t/internal-error-type-does-not-have-a-visitor/6856/2

@bbannier bbannier mentioned this issue Jul 6, 2023
9 tasks
@rsmmr
Copy link
Member

rsmmr commented Aug 3, 2023

Instead of rejecting bitfields here, we should add proper support instead, see the first sketch here.

I'll work on this, planing to make bitfields a full HILTI-side type with corresponding operations.

@rsmmr rsmmr self-assigned this Aug 3, 2023
rsmmr added a commit that referenced this issue Aug 4, 2023
Closes #1032. Using a typedef with bitfields works now.
Closes #1484. `&convert` can now refer to individual bitfields.

TODOs:
    - Like on the HILTI side, no support for &convert and bit order
      yet.
rsmmr added a commit that referenced this issue Aug 7, 2023
Closes #1032. Using a typedef with bitfields works now.
Closes #1484. `&convert` can now refer to individual bitfields.

TODOs:
    - No support for bit order yet.
    - Couple of test failures due to incorrectly failing type comparision.
rsmmr added a commit that referenced this issue Aug 9, 2023
Closes #1032. Using a typedef with bitfields works now.
Closes #1484. `&convert` can now refer to individual bitfields.
rsmmr added a commit that referenced this issue Aug 11, 2023
Closes #1032. Using a typedef with bitfields works now.
Closes #1484. `&convert` can now refer to individual bitfields.
@rsmmr rsmmr closed this as completed in e55296b Aug 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants