Bug #927: Aspectator doesn't ensure uniqueness for auxiliary generated functions - C Instrumentation Framework - Open-Source Projects

Actions

Copy link

Bug #927

closed

Aspectator doesn't ensure uniqueness for auxiliary generated functions

Added by Evgeny Novikov about 13 years ago. Updated over 11 years ago.

Status:

Closed

Priority:

High

Assignee:

Evgeny Novikov

Category:

Start date:

03/11/2011

Due date:

% Done:

Estimated time:

Detected in build:

svn

Platform:

Published in build:

0491d9a

Description

I noticed this when I have obtained more one problem from linking:

Linking globals named 'ldv_module_put_2': symbol multiply defined!

It comes because of different object files to be linked contains the same definition of the given function (and this function hasn't any storage class specifier)! We don't notice the problem earlier since module_put wasn't instrumented (see Bug #362). As for other functions they contain the static storage class specifier for called functions that automatically doesn't lead to the given problem.
I suggest simple obvious fix: always add the static storage class specifier for such auxiliary functions.
Note that there is no problems with actual model functions like ldv_module_put since RI takes care of it.]
The problem should be addressed in the coming aspectator porting.

Related issues 1 (0 open — 1 closed)

Actions

Copy link

Updated by Evgeny Novikov about 13 years ago

Priority changed from High to Normal

Like #349 this issue also isn't a critical now. Even though it's very simple to be implemented.

Actions

Copy link

Updated by Evgeny Novikov over 11 years ago

Priority changed from Normal to High

The given issue wasn't critical so far, but it blocks aspect rerouter! I have noticed strange behavior during instrumentation. It looks like argument signatures are incorrectly exctracted:

ldv_mutex_lock_6(&(media_devnode_lock) /* ldv_func_arg1 */)
{
  ldv_mutex_lock_graph_mutex(ldv_func_arg1 /* lock */)
  {
    assume(ldv_mutex_graph_mutex == 1);
    ldv_mutex_graph_mutex = 2;
    return ;
  }
  mutex_lock(ldv_func_arg1) { /* Function call is skipped due to function is undefined */}
  return ;
}

but indeed this is related with different files defines auxiliary functions with the same signatures:

media-device.c

void ldv_mutex_lock_6 (struct mutex *ldv_func_arg1)
{
  typedef void ldv_func_ret_type;
  typedef struct mutex *ldv_func_arg_type1;

  ldv_mutex_lock_graph_mutex(ldv_func_arg1);

  mutex_lock(ldv_func_arg1);
}

media-devnode.c

void ldv_mutex_lock_6 (struct mutex *ldv_func_arg1)
{
  typedef void ldv_func_ret_type;
  typedef struct mutex *ldv_func_arg_type1;

  ldv_mutex_lock_media_devnode_lock(ldv_func_arg1);

  mutex_lock(ldv_func_arg1);
}

media-entity.c

void ldv_mutex_lock_6 (struct mutex *ldv_func_arg1)
{
  typedef void ldv_func_ret_type;
  typedef struct mutex *ldv_func_arg_type1;

  ldv_mutex_lock_graph_mutex(ldv_func_arg1);

  mutex_lock(ldv_func_arg1);
}

These files are "linked" together by means of CIL that "merges" function definitions having the same signatures and warns users:

Duplicate function skipped due to -ignoredupfn (ldv_mutex_lock_6) 
Duplicate function skipped due to -ignoredupfn (ldv_mutex_lock_6)

I'm going to try solution suggested in the issue description: add static storage class specifier for all auxiliary functions generated.

Actions

Copy link

Updated by Evgeny Novikov over 11 years ago

Adding static and static inline specifiers, and turning off -ignoredupfn didn't help, CIL or BLAST "merges" function with the same signature from different files anyway (with static they warn about duplicates). Of course this is a bug in CIL or/and BLAST implementation but most likely we have to find appropriate solution on the basis of CIF. For instance, we can use a special file to put a current counter value there and use it for a following CIF invocations. Any thoughts?

Actions

Copy link

Updated by Evgeny Novikov over 11 years ago

Status changed from Open to Resolved

I implemented the idea of holding current unique number in a special file in commit 331ae8e of feature-2462 branch. It will be merged to master together with rerouter.

Actions

Copy link

Updated by Evgeny Novikov over 11 years ago

Unfortunately there was a tricky bug in implementation. I noticed that 32_7a model hadn't any unsafe on 'drivers/media' drivers (but should have about 20). It turns out that CIF at the 4th stage didn't exchanged original function names in instrumented function calls with auxiliary ones.
Indeed each stage invokes gcc independently. Just 3d and 4th stages deal with auxiliary functions and require unique numbers. And it's important that both these stage has the same unique numbers to bind original function calls with auxiliary ones. But when 3d stage was completed file holding a current unique number was changed respectively. This led to at the 4th stage the first unique number was the last used at the 3d stage plus 1.
Bug fix is simple. Do not update unique number in the special file during the 3d stage. Commit 68ac0ac of feature-2462 implemented this fix.

Actions

Copy link