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

update @export to handle linksection #2679

Closed
andrewrk opened this issue Jun 14, 2019 · 2 comments · Fixed by #4126
Closed

update @export to handle linksection #2679

andrewrk opened this issue Jun 14, 2019 · 2 comments · Fixed by #4126
Labels
accepted This proposal is planned. breaking Implementing this issue could cause existing code to no longer compile or have different behavior. proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Milestone

Comments

@andrewrk
Copy link
Member

Use case: putting .gdb_script entries in generic types. See also #25.

Here's code that I tried:

fn Generic(comptime T: type) type {
    return struct {
        const python_code = @typeName(@This()) ++
            \\ your generic python
            \\ code goes here
        ;
        extern const script linksection(".debug_gdb_scripts") = "\x04gdb.inlined-script\n" ++ python_code ++ [1]u8{0};
        comptime {
            if (builtin.mode == .Debug) {
                @export("gdb_script" ++ @typeName(@This()), script, .Strong);
            }
        }
    };
}

comptime {
    _ = Generic(i32);
    //_ = Generic(f64);
}

I needed to make a small improvement to @export to support arrays:

--- a/src/ir.cpp
+++ b/src/ir.cpp
@@ -13901,6 +13901,15 @@ static IrInstruction *ir_analyze_instruction_export(IrAnalyze *ira, IrInstructio
                 want_var_export = true;
             }
             break;
+        case ZigTypeIdArray:
+            if (!type_allowed_in_extern(ira->codegen, target->value.type->data.array.child_type)) {
+                ir_add_error(ira, target,
+                    buf_sprintf("array element type '%s' not extern-compatible",
+                        buf_ptr(&target->value.type->data.array.child_type->name)));
+            } else {
+                want_var_export = true;
+            }
+            break;
         case ZigTypeIdMetaType: {
             ZigType *type_value = target->value.data.x_type;
             switch (type_value->id) {
@@ -13968,7 +13977,6 @@ static IrInstruction *ir_analyze_instruction_export(IrAnalyze *ira, IrInstructio
         case ZigTypeIdInt:
         case ZigTypeIdFloat:
         case ZigTypeIdPointer:
-        case ZigTypeIdArray:
         case ZigTypeIdComptimeFloat:
         case ZigTypeIdComptimeInt:
         case ZigTypeIdUndefined:

And then the above example gives me:

/home/andy/downloads/zig/build/test.zig:7:41: error: cannot set section of external variable 'script'
        extern const script linksection(".debug_gdb_scripts") = "\x04gdb.inlined-script\n" ++ python_code ++ [1]u8{0};
                                        ^

This compile error makes sense and should stay. The link section should be a parameter to @export. But we do want the concept of default link sections. We also want strong linkage to be the default. I propose that the definition of @export is changed to this:

fn @export(target: var, comptime options: ExportOptions) void

Where ExportOptions is defined in @import("builtin") like this:

const ExportOptions = struct {
    name: []const u8,
    linkage: GlobalLinkage = .Strong,
    section: ?[]const u8 = null,
};
@andrewrk andrewrk added breaking Implementing this issue could cause existing code to no longer compile or have different behavior. proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. labels Jun 14, 2019
@andrewrk andrewrk added this to the 0.6.0 milestone Jun 14, 2019
@andrewrk
Copy link
Member Author

Oh, actually this compile error should go away:

External declarations may have an explicit section specified. Section information is retained in LLVM IR for targets that make use of this information. Attaching section information to an external declaration is an assertion that its definition is located in the specified section. If the definition is located in a different section, the behavior is undefined.

I'm still in favor of this proposal (maybe we make an error for if the sections disagree) but in the meantime the compile error for section names on extern variables can go away.

andrewrk added a commit that referenced this issue Jun 14, 2019
previously `@export` for an array would panic with a TODO message.
now it will do the export. However, it uses the variable's name
rather than the name passed to `@export`. Issue #2679 remains open
for that problem.
andrewrk added a commit that referenced this issue Jun 14, 2019
Previously, the symbol name parameter of `@export` would be ignored for
variables, and the variable name would be used for the symbol name.
Now it works as expected.

See #2679
@andrewrk
Copy link
Member Author

I fixed the issue with @export not respecting the name. So this example now works (See the LLVM IR output below):

// This is to remove the SHF_ALLOC flag from the .gdb_debug_scripts section so that it does not get garbage collected by the linker.
comptime {
    asm (
        \\.pushsection ".debug_gdb_scripts", "MS",@progbits,1
        \\.popsection
    );
}

fn Generic(comptime T: type) type {
    return struct {
        const python_code = @typeName(@This()) ++
            \\ your generic python
            \\ code goes here
        ;
        // note that you have to make this an extern for now
        extern const script linksection(".debug_gdb_scripts") = "\x04gdb.inlined-script\n" ++ python_code ++ [1]u8{0};
        comptime {
            if (builtin.mode == .Debug) {
                @export("gdb_script" ++ @typeName(@This()), script, .Strong);
            }
        }
    };
}

comptime {
    _ = Generic(i32);
    _ = Generic(f64);
}

Relevant LLVM output:

@"gdb_scriptGeneric(i32)" = dso_local unnamed_addr constant [69 x i8] c"\04gdb.inlined-script\0AGeneric(i32) your generic python\0A code goes here\00", section ".debug_gdb_scripts", align 1
@"gdb_scriptGeneric(f64)" = dso_local unnamed_addr constant [69 x i8] c"\04gdb.inlined-script\0AGeneric(f64) your generic python\0A code goes here\00", section ".debug_gdb_scripts", align 1

This will unblock #2681, which should not wait on this issue to be solved, because it will remain open until @export has the new function signature.

@andrewrk andrewrk modified the milestones: 0.6.0, 0.7.0 Jan 8, 2020
LemonBoy added a commit to LemonBoy/zig that referenced this issue Jan 9, 2020
Use a struct as second parameter to be future proof (and also allows to
specify default values for the parameters)

Closes ziglang#2679 as it was just a matter of a few lines of code.
LemonBoy added a commit to LemonBoy/zig that referenced this issue Jan 9, 2020
Use a struct as second parameter to be future proof (and also allows to
specify default values for the parameters)

Closes ziglang#2679 as it was just a matter of a few lines of code.
@andrewrk andrewrk added the accepted This proposal is planned. label Jan 9, 2020
andrewrk pushed a commit that referenced this issue Jan 9, 2020
Use a struct as second parameter to be future proof (and also allows to
specify default values for the parameters)

Closes #2679 as it was just a matter of a few lines of code.
@andrewrk andrewrk modified the milestones: 0.7.0, 0.6.0 Feb 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted This proposal is planned. breaking Implementing this issue could cause existing code to no longer compile or have different behavior. proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant