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

Can't take address of extern lib variables #4845

Closed
bew opened this issue Aug 17, 2017 · 9 comments · Fixed by #5398
Closed

Can't take address of extern lib variables #4845

bew opened this issue Aug 17, 2017 · 9 comments · Fixed by #5398

Comments

@bew
Copy link
Contributor

bew commented Aug 17, 2017

lib Lib
  $extern : Int32
end

puts pointerof(Lib.extern)

Error: can't take address of Lib.extern (https://carc.in/#/r/2jvw)

As pointed out by @oprypin, I can make a copy, then take the pointer of that copy, but this workaround has one issue: the pointer of the copy and the pointer of the original extern variable won't match, and it can be a problem in some lib.

@bew
Copy link
Contributor Author

bew commented Aug 18, 2017

The error comes from main_visitor#L2417. There, node_exp (Lib.extern) is a Call.
I currently didn't found a way to detect that this call is a lib-extern variable access..

@ysbaddaden
Copy link
Contributor

What's your use-case?

@bew
Copy link
Contributor Author

bew commented Aug 18, 2017

Basically, I have the following schema:

lib L
  # singleton provided by the lib
  $singleton_global_object : Obj

  struct Obj
    field : UInt8*
    # more fields...
  end

  fun fetch_something_using_obj(obj : Obj*) : Int32
end

puts L.fetch_something_using_obj(pointerof(L.singleton_global_object))

I could use my own Obj to "fetch something" but the lib provides multiple reference global objects Obj for different usages.

@ysbaddaden
Copy link
Contributor

And the C API doesn't provide any method to get a pointer to that singleton (BTW: that's ugly, thread unsafe, ...)?

@RX14
Copy link
Contributor

RX14 commented Aug 18, 2017

I can see being able to get a pointer at arbitrary C symbols being useful. I also don't see why this shouldn't work regardless of whether it's actually good practice.

@bew
Copy link
Contributor Author

bew commented Aug 18, 2017

And the C API doesn't provide any method to get a pointer to that singleton ?

Currently no, but the lib is still in development (it's wayland), maybe they'll add this later

@ysbaddaden
Copy link
Contributor

I ended up having this issue myself. I needed to access the address of some linker symbols (namely __data_start and _end) but it's currently impossible in Crystal. In C it would be:

extern char __data_start[];
void *addr = &__data_start;

@larubujo
Copy link
Contributor

i think this patch implements feature. but lazy to submit pr. tested with LibC.environ.

diff --git a/spec/compiler/codegen/pointer_spec.cr b/spec/compiler/codegen/pointer_spec.cr
index e4022ce1f..d4514a507 100644
--- a/spec/compiler/codegen/pointer_spec.cr
+++ b/spec/compiler/codegen/pointer_spec.cr
@@ -474,4 +474,14 @@ describe "Code gen: pointer" do
       ptr == ptr
       )).to_b.should be_true
   end
+
+  it "takes pointerof lib external var" do
+    codegen(%(
+      lib LibFoo
+        $extern : Int32
+      end
+
+      pointerof(LibFoo.extern)
+      ))
+  end
 end
diff --git a/spec/compiler/semantic/pointer_spec.cr b/spec/compiler/semantic/pointer_spec.cr
index b599633c1..4df32cb6c 100644
--- a/spec/compiler/semantic/pointer_spec.cr
+++ b/spec/compiler/semantic/pointer_spec.cr
@@ -169,4 +169,14 @@ describe "Semantic: pointer" do
       ),
       "can't create instance of a pointer type"
   end
+
+  it "takes pointerof lib external var" do
+    assert_type(%(
+      lib LibFoo
+        $extern : Int32
+      end
+
+      pointerof(LibFoo.extern)
+      )) { pointer_of(int32) }
+  end
 end
diff --git a/src/compiler/crystal/codegen/codegen.cr b/src/compiler/crystal/codegen/codegen.cr
index 359c65fa0..cbf63adb7 100644
--- a/src/compiler/crystal/codegen/codegen.cr
+++ b/src/compiler/crystal/codegen/codegen.cr
@@ -492,6 +492,11 @@ module Crystal
               when ReadInstanceVar
                 node_exp.obj.accept self
                 instance_var_ptr (node_exp.obj.type), node_exp.name, @last
+              when Call
+                # lib external var
+                extern = node_exp.dependencies.first.as(External)
+                var = get_external_var(extern)
+                check_c_fun extern.type, var
               else
                 raise "BUG: #{node}"
               end
diff --git a/src/compiler/crystal/codegen/primitives.cr b/src/compiler/crystal/codegen/primitives.cr
index 8652cfe8e..1a1925153 100644
--- a/src/compiler/crystal/codegen/primitives.cr
+++ b/src/compiler/crystal/codegen/primitives.cr
@@ -557,8 +557,7 @@ class Crystal::CodeGenVisitor
 
   def codegen_primitive_external_var_get(node, target_def, call_args)
     external = target_def.as(External)
-    name = target_def.as(External).real_name
-    var = declare_lib_var name, node.type, external.thread_local?
+    var = get_external_var(external)
 
     if external.type.passed_by_value?
       @last = var
@@ -571,7 +570,12 @@ class Crystal::CodeGenVisitor
     @last
   end
 
-  def codegen_primitive_object_id(node, target_def, call_args)
+  def get_external_var(external)
+    name = external.as(External).real_name
+    declare_lib_var name, external.type, external.thread_local?
+  end
+
+  def codegen_primitive_object_id(node, external, call_args)
     ptr2int call_args[0], llvm_context.int64
   end
 
diff --git a/src/compiler/crystal/semantic/main_visitor.cr b/src/compiler/crystal/semantic/main_visitor.cr
index 333b881ff..fdfe9ffe1 100644
--- a/src/compiler/crystal/semantic/main_visitor.cr
+++ b/src/compiler/crystal/semantic/main_visitor.cr
@@ -2400,34 +2400,48 @@ module Crystal
     end
 
     def visit(node : PointerOf)
-      var = case node_exp = node.exp
-            when Var
-              meta_var = @meta_vars[node_exp.name]
-              meta_var.assigned_to = true
-              meta_var
-            when InstanceVar
-              lookup_instance_var node_exp
-            when ClassVar
-              visit_class_var node_exp
-            when Global
-              visit_global node_exp
-            when Path
-              node_exp.accept self
-              if const = node_exp.target_const
-                const.value
-              else
-                node_exp.raise "can't take address of #{node_exp}"
-              end
-            when ReadInstanceVar
-              visit_read_instance_var(node_exp)
-              node_exp
-            else
-              node_exp.raise "can't take address of #{node_exp}"
-            end
+      var = pointerof_var(node)
+      node.exp.raise "can't take address of #{node.exp}" unless var
       node.bind_to var
       true
     end
 
+    def pointerof_var(node)
+      case exp = node.exp
+      when Var
+        meta_var = @meta_vars[exp.name]
+        meta_var.assigned_to = true
+        meta_var
+      when InstanceVar
+        lookup_instance_var exp
+      when ClassVar
+        visit_class_var exp
+      when Global
+        visit_global exp
+      when Path
+        exp.accept self
+        if const = exp.target_const
+          const.value
+        end
+      when ReadInstanceVar
+        visit_read_instance_var(exp)
+        exp
+      when Call
+        # Check lib external var
+        return unless exp.args.empty? && !exp.named_args && !exp.block && !exp.block_arg
+
+        obj = exp.obj
+        return unless obj
+
+        obj.accept(self)
+
+        obj_type = obj.type?
+        return unless obj_type.is_a?(LibType)
+
+        obj_type.lookup_var(exp.name)
+      end
+    end
+
     def visit(node : TypeOf)
       # A typeof shouldn't change the type of variables:
       # so we keep the ones before it and restore them at the end
diff --git a/src/compiler/crystal/types.cr b/src/compiler/crystal/types.cr
index fe89dd6b1..3334453ff 100644
--- a/src/compiler/crystal/types.cr
+++ b/src/compiler/crystal/types.cr
@@ -2286,6 +2286,16 @@ module Crystal
       add_def getter
     end
 
+    def lookup_var(name)
+      a_def = lookup_first_def(name, false)
+      return nil unless a_def
+
+      body = a_def.body
+      return nil unless body.is_a?(Primitive) && body.name == "external_var_get"
+
+      a_def
+    end
+
     def type_desc
       "lib"
     end

@bew
Copy link
Contributor Author

bew commented Dec 20, 2017

Awesome @larubujo, thanks!

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.

5 participants