Re: [PATCH dwarves 2/4] dwarf_loader: add "artificial" and "top_level" variable flags

[Alan Maguire <alan.maguire@oracle.com>]

On 12/09/2024 20:08, Stephen Brennan wrote:
> The "artificial" flag corresponds directly to DW_AT_artificial, which
> indicates a compiler-generated variable (e.g. __func__) which shouldn't
> be included in the output.
>

Nice, didn't know about this! Great to have such a clear criterion for
filtering.

> The "top_level" flag is intended to be a better proxy for global scoped
> variables. Currently, the DWARF loader examines the DWARF location

Looking at this, it appears that top_level means it is a top-level
compilation unit tag, i.e. not associated with a subroutine tag (I
think?). That's kind of implicit in your explanation so I think it would
be helpful for the log to describe what it is as well as how you're
using it.

> expression, and if the location is found to be at a constant memory
> address (not stack, register, etc), then the variable is assumed to be
> globally scoped. However, this includes a variety of variables that
> aren't truly globally scoped: most commonly, static variables in
> functions definitions. Their locations may be static, but they're not
> globally accessible in any useful way.
> 
> These flags will be used by the BTF encoder to select global variables.
> 
> Signed-off-by: Stephen Brennan <stephen.s.brennan@oracle.com>


Reviewed-by: Alan Maguire <alan.maguire@oracle.com>

> ---
>  dwarf_loader.c | 12 +++++++-----
>  dwarves.h      |  2 ++
>  2 files changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/dwarf_loader.c b/dwarf_loader.c
> index 065ed4d..d162214 100644
> --- a/dwarf_loader.c
> +++ b/dwarf_loader.c
> @@ -730,7 +730,7 @@ const char *variable__scope_str(const struct variable *var)
>  	return "unknown";
>  }
>  
> -static struct variable *variable__new(Dwarf_Die *die, struct cu *cu, struct conf_load *conf)
> +static struct variable *variable__new(Dwarf_Die *die, struct cu *cu, struct conf_load *conf, int top_level)
>  {
>  	bool has_specification = dwarf_hasattr(die, DW_AT_specification);
>  	struct variable *var = tag__alloc(cu, sizeof(*var));
> @@ -743,6 +743,8 @@ static struct variable *variable__new(Dwarf_Die *die, struct cu *cu, struct conf
>  		/* non-defining declaration of an object */
>  		var->declaration = dwarf_hasattr(die, DW_AT_declaration);
>  		var->has_specification = has_specification;
> +		var->artificial = dwarf_hasattr(die, DW_AT_artificial);
> +		var->top_level = top_level;
>  		var->scope = VSCOPE_UNKNOWN;
>  		INIT_LIST_HEAD(&var->annots);
>  		var->ip.addr = 0;
> @@ -1767,9 +1769,9 @@ static struct tag *die__create_new_label(Dwarf_Die *die,
>  	return &label->ip.tag;
>  }
>  
> -static struct tag *die__create_new_variable(Dwarf_Die *die, struct cu *cu, struct conf_load *conf)
> +static struct tag *die__create_new_variable(Dwarf_Die *die, struct cu *cu, struct conf_load *conf, int top_level)
>  {
> -	struct variable *var = variable__new(die, cu, conf);
> +	struct variable *var = variable__new(die, cu, conf, top_level);
>  
>  	if (var == NULL || add_child_llvm_annotations(die, -1, conf, &var->annots))
>  		return NULL;
> @@ -2243,7 +2245,7 @@ static int die__process_function(Dwarf_Die *die, struct ftype *ftype,
>  			tag = die__create_new_parameter(die, ftype, lexblock, cu, conf, param_idx++);
>  			break;
>  		case DW_TAG_variable:
> -			tag = die__create_new_variable(die, cu, conf);
> +			tag = die__create_new_variable(die, cu, conf, 0);
>  			if (tag == NULL)
>  				goto out_enomem;
>  			lexblock__add_variable(lexblock, tag__variable(tag));
> @@ -2367,7 +2369,7 @@ static struct tag *__die__process_tag(Dwarf_Die *die, struct cu *cu,
>  	case DW_TAG_union_type:
>  		tag = die__create_new_union(die, cu, conf);	break;
>  	case DW_TAG_variable:
> -		tag = die__create_new_variable(die, cu, conf);	break;
> +		tag = die__create_new_variable(die, cu, conf, top_level);	break;
>  	case DW_TAG_constant: // First seen in a Go CU
>  		tag = die__create_new_constant(die, cu, conf);	break;
>  	default:
> diff --git a/dwarves.h b/dwarves.h
> index f2d3988..0fede91 100644
> --- a/dwarves.h
> +++ b/dwarves.h
> @@ -848,6 +848,8 @@ struct variable {
>  	uint8_t		 external:1;
>  	uint8_t		 declaration:1;
>  	uint8_t		 has_specification:1;
> +	uint8_t		 artificial:1;
> +	uint8_t		 top_level:1;
>  	enum vscope	 scope;
>  	struct location	 location;
>  	struct hlist_node tool_hnode;