Merge remote-tracking branch 'upstream/master'

SiyuanSun0736 · SiyuanSun0736 · commit 6633a829ee6a · 2026-03-22T14:41:23.000+08:00
diff --git a/src/ebpf_c_codegen.ml b/src/ebpf_c_codegen.ml
@@ -2581,7 +2581,10 @@ and generate_assignment ctx dest_val expr is_const =
              | IRValue src_val ->
                  (* Simple value assignment *)
                  let dest_str = generate_c_value ctx dest_val in
-                 let src_str = generate_c_value ctx src_val in
+                 (* Auto-dereference map access to get the value, not the pointer *)
+                 let src_str = (match src_val.value_desc with
+                   | IRMapAccess (_, _, _) -> generate_c_value ~auto_deref_map_access:true ctx src_val
+                   | _ -> generate_c_value ctx src_val) in
                  emit_line ctx (sprintf "%s%s = %s;" assignment_prefix dest_str src_str)
              | _ ->
                  (* Other expressions *)
diff --git a/src/ir_generator.ml b/src/ir_generator.ml
@@ -2825,12 +2825,15 @@ let lower_multi_program ast symbol_table source_name =
           let ir_map = lower_map_declaration symbol_table map_decl in
           ((decl, `Map ir_map) :: decls, ir_map :: maps, vars)
       | Ast.GlobalVarDecl var_decl when is_global_var_map var_decl ->
+          (* When a global variable has a map type, convert it to a map definition only.
+             Do NOT create a corresponding IR global variable, as that causes the map
+             to be incorrectly treated as a pinned global variable in userspace codegen. *)
           let ir_map = match convert_global_var_to_map symbol_table var_decl with
             | Some map -> map
             | None -> failwith "Expected map conversion to succeed"
           in
-          let ir_var = lower_global_variable_declaration symbol_table var_decl in
-          ((decl, `MapFromGlobalVar (ir_map, ir_var)) :: decls, ir_map :: maps, ir_var :: vars)
+          (* Only track as map, not as global variable *)
+          ((decl, `MapFromGlobalVar ir_map) :: decls, ir_map :: maps, vars)
       | Ast.GlobalVarDecl var_decl ->
           let ir_var = lower_global_variable_declaration symbol_table var_decl in
           ((decl, `GlobalVar ir_var) :: decls, maps, ir_var :: vars)
@@ -3136,10 +3139,11 @@ let lower_multi_program ast symbol_table source_name =
     match processed with
     | `Map ir_map -> 
         add_source_declaration (IRDeclMapDef ir_map) pos
-    | `MapFromGlobalVar (ir_map, ir_var) ->
-        (* For global variables with map types, add both the map and the variable *)
-        add_source_declaration (IRDeclMapDef ir_map) pos;
-        add_source_declaration (IRDeclGlobalVarDef ir_var) pos
+    | `MapFromGlobalVar ir_map ->
+        (* For global variables with map types, only add the map definition.
+           Do NOT add a global variable declaration, as it would incorrectly be treated
+           as a pinned global variable in userspace codegen. *)
+        add_source_declaration (IRDeclMapDef ir_map) pos
     | `GlobalVar ir_var ->
         add_source_declaration (IRDeclGlobalVarDef ir_var) pos
     | `Other decl ->
diff --git a/src/userspace_codegen.ml b/src/userspace_codegen.ml
@@ -2610,7 +2610,13 @@ let generate_c_function_from_ir ?(global_variables = []) ?(base_name = "") ?(con
     let pinned_globals_vars = List.filter (fun gv -> gv.is_pinned) global_variables in
     let has_pinned_globals = List.length pinned_globals_vars > 0 in
     
-    let setup_call = if needs_object_loading && (List.length config_declarations > 0 || func_usage.uses_map_operations || func_usage.uses_exec || has_pinned_globals) then
+    (* Check if there are any pinned maps that need setup *)
+    let has_pinned_maps = match ir_multi_prog with
+      | Some multi_prog -> List.exists (fun map -> map.pin_path <> None) (Ir.get_global_maps multi_prog)
+      | None -> false
+    in
+    
+    let setup_call = if needs_object_loading && (List.length config_declarations > 0 || func_usage.uses_map_operations || func_usage.uses_exec || has_pinned_globals || has_pinned_maps) then
       let all_setup_parts = List.filter (fun s -> s <> "") [
         (if has_pinned_globals then
           let project_name = base_name in
@@ -2637,8 +2643,8 @@ let generate_c_function_from_ir ?(global_variables = []) ?(base_name = "") ?(con
         }
     }|} pin_path pin_path
                 else "");
-        (* Include all_setup_code only once for maps, config, struct_ops, and ringbuf *)
-        (if func_usage.uses_map_operations || func_usage.uses_exec || List.length config_declarations > 0 then all_setup_code else "");
+        (* Include all_setup_code for maps (including pinned maps), config, struct_ops, and ringbuf *)
+        (if func_usage.uses_map_operations || func_usage.uses_exec || List.length config_declarations > 0 || has_pinned_maps then all_setup_code else "");
         ] in
       if all_setup_parts <> [] then "\n" ^ String.concat "\n" all_setup_parts else ""
     else "" in
@@ -2811,6 +2817,19 @@ let generate_pinned_globals_support _project_name global_variables =
 
 (** Generate ring buffer event handler functions *)
 let generate_ringbuf_handlers_from_registry (registry : Ir.ir_ring_buffer_registry) ~dispatch_used =
+  (* Generate forward declarations for callback functions *)
+  let forward_declarations = List.map (fun rb_decl ->
+    let ringbuf_name = rb_decl.rb_name in
+    let value_type = c_type_from_ir_type rb_decl.rb_value_type in
+    let handler_name = match List.assoc_opt ringbuf_name registry.event_handler_registrations with
+      | Some handler -> handler
+      | None -> 
+          (* Try callback function naming convention: {ringbuf_name}_callback *)
+          ringbuf_name ^ "_callback"
+    in
+    sprintf "int %s(%s *event);" handler_name value_type
+  ) registry.ring_buffer_declarations |> String.concat "\n" in
+  
   let event_handlers = List.map (fun rb_decl ->
     let ringbuf_name = rb_decl.rb_name in
     let value_type = c_type_from_ir_type rb_decl.rb_value_type in
@@ -2836,7 +2855,11 @@ static int %s_event_handler(void *ctx, void *data, size_t data_sz) {
   else "" in
   
   (* Only generate event handlers if dispatch is actually used *)
-  let final_event_handlers = if dispatch_used then event_handlers else "" in
+  let final_event_handlers = if dispatch_used then 
+    if List.length registry.ring_buffer_declarations > 0 then
+      sprintf "\n// Forward declarations for ring buffer callbacks\n%s\n%s" forward_declarations event_handlers
+    else ""
+  else "" in
   
   final_event_handlers ^ combined_rb_declaration
 
@@ -2957,23 +2980,30 @@ let generate_unified_map_setup_code ?(obj_var="obj->obj") maps =
     else map :: acc
   ) [] maps |> List.rev in
   
-  List.map (fun map ->
+  let map_setups = List.map (fun map ->
     (* Always load from eBPF object first, then handle pinning if needed *)
     let pin_logic = match map.pin_path with
       | Some pin_path ->
+          (* Extract directory path from pin_path *)
+          let dir_path = Filename.dirname pin_path in
+          (* Generate unique variable name for each map's existing_fd *)
           Printf.sprintf {|
         // Check if map is already pinned
-        int existing_fd = bpf_obj_get("%s");
-        if (existing_fd >= 0) {
-            %s_fd = existing_fd;
+        int %s_existing_fd = bpf_obj_get("%s");
+        if (%s_existing_fd >= 0) {
+            %s_fd = %s_existing_fd;
         } else {
-            // Map not pinned yet, pin it now
+            // Map not pinned yet, create directory and pin it
+            if (ensure_bpf_dir("%s") < 0) {
+                fprintf(stderr, "Failed to create directory %s: %%s\n", strerror(errno));
+                return 1;
+            }
             if (bpf_map__pin(%s_map, "%s") < 0) {
                 fprintf(stderr, "Failed to pin %s map to %s\n");
                 return 1;
             }
             %s_fd = bpf_map__fd(%s_map);
-        }|} pin_path map.map_name map.map_name pin_path map.map_name pin_path map.map_name map.map_name
+        }|} map.map_name pin_path map.map_name map.map_name map.map_name dir_path dir_path map.map_name pin_path map.map_name pin_path map.map_name map.map_name
       | None ->
           Printf.sprintf {|
         // Non-pinned map, just get file descriptor
@@ -2989,7 +3019,9 @@ let generate_unified_map_setup_code ?(obj_var="obj->obj") maps =
         fprintf(stderr, "Failed to get fd for %s map\n");
         return 1;
     }|} map.map_name map.map_name obj_var map.map_name map.map_name map.map_name pin_logic map.map_name map.map_name
-  ) deduplicated_maps |> String.concat "\n"
+  ) deduplicated_maps in
+  
+  String.concat "\n" map_setups
 
 (** Generate config struct definition from config declaration - reusing eBPF logic *)
 let generate_config_struct_from_decl (config_decl : Ast.config_declaration) =
@@ -3446,8 +3478,14 @@ let generate_complete_userspace_program_from_ir ?(config_declarations = []) ?(ta
     maps_for_exec  (* Use all global maps directly for exec *)
   else used_global_maps in
 
+  (* Check if there are any pinned maps - this affects which headers we need *)
+  let has_any_pinned_maps = List.exists (fun map -> map.pin_path <> None) global_maps in
+  
+  (* For header generation, use all global maps if there are pinned maps, otherwise use the filtered list *)
+  let maps_for_headers = if has_any_pinned_maps then global_maps else used_global_maps_with_exec in
+  
   let uses_bpf_functions = all_usage.uses_load || all_usage.uses_attach || all_usage.uses_detach in
-  let base_includes = generate_headers_for_maps ~uses_bpf_functions used_global_maps_with_exec in
+  let base_includes = generate_headers_for_maps ~uses_bpf_functions maps_for_headers in
   let additional_includes = {|#include <stdbool.h>
 #include <stdint.h>
 #include <inttypes.h>
@@ -3520,8 +3558,12 @@ let generate_complete_userspace_program_from_ir ?(config_declarations = []) ?(ta
   else "" in
   
   (* Generate setup code first for use in main function *)
-  let map_setup_code = if all_usage.uses_map_operations || all_usage.uses_exec then
-    generate_unified_map_setup_code used_global_maps_with_exec
+  (* Check if there are any pinned maps that need setup *)
+  let has_pinned_maps = List.exists (fun map -> map.pin_path <> None) global_maps in
+  let map_setup_code = if all_usage.uses_map_operations || all_usage.uses_exec || has_pinned_maps then
+    (* For pinned maps, we need to include all of them in setup, not just used ones *)
+    let maps_for_setup = if has_pinned_maps then global_maps else used_global_maps_with_exec in
+    generate_unified_map_setup_code maps_for_setup
   else "" in
   
   (* Generate pinned globals support *)
@@ -3588,8 +3630,9 @@ let generate_complete_userspace_program_from_ir ?(config_declarations = []) ?(ta
   
 
   
-  let map_fd_declarations = if all_usage.uses_map_operations || all_usage.uses_exec then
-    generate_map_fd_declarations used_global_maps_with_exec
+  let map_fd_declarations = if all_usage.uses_map_operations || all_usage.uses_exec || has_pinned_maps then
+    let maps_for_fd = if has_pinned_maps then global_maps else used_global_maps_with_exec in
+    generate_map_fd_declarations maps_for_fd
   else "" in
   
   (* Generate config map file descriptors if there are config declarations *)
@@ -3666,6 +3709,9 @@ void cleanup_bpf_maps(void) {
   
   (* Only generate BPF helper functions when they're actually used *)
   let bpf_helper_functions = 
+    (* Check if there are any pinned maps in the global maps *)
+    let has_pinned_maps = List.exists (fun map -> map.pin_path <> None) global_maps in
+    
     let load_function = generate_load_function_with_tail_calls base_name all_usage tail_call_analysis all_setup_code kfunc_dependencies (Ir.get_global_variables ir_multi_prog) in
     
     (* Global attachment storage (generated only when attach/detach are used) *)
@@ -4140,7 +4186,40 @@ static int add_attachment(int prog_fd, const char *target, uint32_t flags,
         ) maps_for_exec |> String.concat "\n")
     else "" in
 
-    let functions_list = List.filter (fun s -> s <> "") [attachment_storage; load_function; attach_function; detach_function; daemon_function; exec_function] in
+    (* Generate directory creation helper if there are pinned maps *)
+    let mkdir_helper_function = if has_pinned_maps then
+      {|// Helper function to create directory recursively
+static int ensure_bpf_dir(const char *path) {
+    char tmp[4096];
+    char *p = NULL;
+    size_t len;
+    
+    if (!path || strlen(path) >= sizeof(tmp)) {
+        fprintf(stderr, "ensure_bpf_dir: path too long or NULL\n");
+        return -1;
+    }
+    
+    snprintf(tmp, sizeof(tmp), "%s", path);
+    len = strlen(tmp);
+    if (len > 0 && tmp[len - 1] == '/') tmp[len - 1] = 0;
+    
+    for (p = tmp + 1; *p; p++) {
+        if (*p == '/') {
+            *p = 0;
+            if (mkdir(tmp, 0755) != 0 && errno != EEXIST) {
+                return -1;
+            }
+            *p = '/';
+        }
+    }
+    if (mkdir(tmp, 0755) != 0 && errno != EEXIST) {
+        return -1;
+    }
+    return 0;
+}|}
+    else "" in
+
+    let functions_list = List.filter (fun s -> s <> "") [mkdir_helper_function; attachment_storage; load_function; attach_function; detach_function; daemon_function; exec_function] in
     if functions_list = [] && bpf_obj_decl = "" then ""
     else
       sprintf "\n/* BPF Helper Functions (generated only when used) */\n%s\n\n%s" 
diff --git a/tests/test_ebpf_c_codegen.ml b/tests/test_ebpf_c_codegen.ml
@@ -1146,6 +1146,37 @@ let test_global_map_redefinition_fix () =
   
   ()
 
+(** Test map access auto-dereference in variable assignment (covers lines 2559-2566) *)
+let test_map_access_auto_deref_in_assignment () =
+  let ctx = create_c_context () in
+
+  (* Build: count = my_map[user_key]
+     IRMapAccess carries the raw lookup-pointer as its underlying value. *)
+  let key_val = make_ir_value (IRVariable "user_key") IRU32 test_pos in
+  let underlying_desc = IRVariable "map_ptr_0" in
+  let underlying_type = IRPointer (IRU64, make_bounds_info ()) in
+  let src_val = make_ir_value
+    (IRMapAccess ("my_map", key_val, (underlying_desc, underlying_type)))
+    IRU64 test_pos in
+
+  let dest_val = make_ir_value (IRVariable "count") IRU64 test_pos in
+  let assign_instr = make_ir_instruction
+    (IRAssign (dest_val, make_ir_expr (IRValue src_val) IRU64 test_pos))
+    test_pos in
+
+  generate_c_instruction ctx assign_instr;
+
+  let output = String.concat "\n" ctx.output_lines in
+
+  (* auto_deref_map_access:true emits a guarded dereference:
+     count = ({ __u64 __val = {0}; if (map_ptr_0) { __val = *(map_ptr_0); } __val; }); *)
+  check bool "dest variable present in output" true (contains_substr output "count =");
+  check bool "__val used for safe dereference" true (contains_substr output "__val");
+  check bool "null-guard if-check emitted" true (contains_substr output "if (map_ptr_0)");
+  check bool "pointer dereference emitted" true (contains_substr output "*(map_ptr_0)");
+  (* Without the fix this branch would fall through to the raw-pointer path *)
+  check bool "no raw pointer assignment" false (contains_substr output "count = map_ptr_0")
+
 (** Tests for tail call fallback return fix.
     These tests verify that every match arm containing a tail call emits an
     explicit fallback return statement so the eBPF verifier can confirm that
@@ -1305,6 +1336,8 @@ let suite =
     ("eBPF function generation bug fix", `Quick, test_ebpf_function_generation_bug_fix);
     (* Test to prevent global variable map redefinition regression *)
     ("Global map redefinition fix", `Quick, test_global_map_redefinition_fix);
+    (* Coverage for IRMapAccess auto-dereference path in generate_assignment *)
+    ("Map access auto-deref in assignment", `Quick, test_map_access_auto_deref_in_assignment);
     (* Tail call fallback return fix - verifier requires explicit return after bpf_tail_call() *)
     ("Tail call fallback: constant arm XDP context", `Quick, test_tail_call_fallback_constant_arm_xdp);
     ("Tail call fallback: default arm TC context",   `Quick, test_tail_call_fallback_default_arm_tc);
diff --git a/tests/test_ringbuf.ml b/tests/test_ringbuf.ml
diff --git a/tests/test_userspace_maps.ml b/tests/test_userspace_maps.ml
