API details
sunfishcode opened this issue · 7 comments
If I declare a symbol as a DataImport
and then later declare it as a Data
, like this:
let mut obj = Artifact::new(Target::X86_64, Some("t.o".into()));
obj.declare("str.0", faerie::Decl::DataImport {}).expect(
"declare",
);
obj.declare(
&"str.0".to_string(),
faerie::Decl::Data {
global: false,
writeable: false,
},
).expect("declare");
obj.define("str.0", "hello world\0".to_string().into_bytes())
.expect("define");
I get an error:
thread 'main' panicked at 'declare: DuplicateDeclaration("str.0")', /checkout/src/libcore/result.rs:916:4
If I reverse the order of the Data
and DataImport
declarations, it looks like the DataImport
silently wins; I get:
thread 'main' panicked at 'define: ImportDefined("str.0")', /checkout/src/libcore/result.rs:916:4
I kind of expect faerie to accept this, with a Data
declaration overriding a DataImport
, regardless of which comes first.
Possibly the source of my expectation is C, where code like this:
extern int x;
int x = 2;
extern int x;
compiles and the definition "wins". But also, I would find it convenient in some situations, because while I usually know up front the set of things that will be defined, I don't always know up front what's going to be imported unless I do some extra work.
Wouldn't a more accurate C version of the above be something:
extern int x;
int x = 2;
extern float x;
int main() {
return 0;
}
which does not compile with either clang or gcc (as expected):
test.c:3:14: error: conflicting types for ‘x’
extern float x;
^
test.c:2:5: note: previous definition of ‘x’ was here
int x = 2;
^
m4b@efrit :: [ /tmp ] clang test.c
test.c:3:14: error: redeclaration of 'x' with a different type: 'float' vs 'int'
extern float x;
^
test.c:2:5: note: previous definition is here
int x = 2;
^
1 error generated.
We can easily ignore duplicate exact declarations; but imho declaring the same symbol with different types and semantics, just seems like a fundamental compiler error, no?
For cases like:
extern int x;
int x = 2;
int main() {
return 0;
}
which would be an import decl and then a definition, we could relax this for imports and allow the definition to take place, when the types exactly match (and subsequent import calls are no-ops).
But anything else just seems like the wild west to me, but I dunno :P
Ah i see now you had Data, for some reason reading it on my phone I thought it was a data and then a cstr declaration; yes I agree, behavior when the declaration matches the definition should be C-ish, basically:
- Duplicate declarations of same type are no-ops
- Declaration, definition must match as usual, OR decl is an import and same datatype
- definition, declaration, when types match OR its an import decl are no ops.
I think these are the rules you want, yes?
Yes, that sounds right.
98cb29f caused a regression :/
11: faerie::elf::Elf::link
at src/elf.rs:464
12: <faerie::elf::Elf<'a> as faerie::artifact::Object>::to_bytes
at src/elf.rs:671
13: faerie::artifact::Artifact::emit
at ./src/artifact.rs:384
14: faerie::artifact::Artifact::write
at ./src/artifact.rs:388
15: prototype::run
at src/bin/main.rs:128
16: prototype::main
at src/bin/main.rs:178
17: __rust_maybe_catch_panic
at /checkout/src/libpanic_unwind/lib.rs:101
18: std::rt::lang_start
at /checkout/src/libstd/panicking.rs:459
at /checkout/src/libstd/rt.rs:58
19: main
20: __libc_start_main
21: _start
heh; i have absolutely no idea why/how this could have caused a regression at this point ;)
So the problem was I deleted the import updating logic 😭
A lot of this will be caught now, as the CI runs the tests, runs the prototype to build a deadbeef.o, then runs it again on the code similar to whats in readme, linked with deadbeef.o, and then invokes the linker on the result, and then runs the resulting binary.