Re: Plugin API Comments (was Re: GCC Plug-in Framework ready to port)
Hi Sean,
It's great that you updated the wiki page with the latest and more
detailed API design.
We (at Google) also started to look at the GCC plugin support a couple
of weeks ago. We had a quick prototype implemented based on the
original APIs that Taras put together in the old wiki. (I can send out
the patch later for people's reference.) The updated APIs in general
look good to me. I do have some comments based on our experience with
the initial prototyping:
>>
>> void register_callbacks(int nregistrations, struct plugin_registration
>> *registrations);
>>
Instead of passing in an array of plugin_registration objects with a
single register_callbacks call, it's probably better to have the
plugin call a sequence of register_callback with the necessary
information, as shown in the following example:
void plugin_register_callback (const char *plugin_name, enum
plugin_event event, plugin_callback_func callback, void *user_data);
/* In plugin code */
void
plugin_init()
{
...
register_callback (plugin_name, PLUGIN_FINISH_STRUCT, handle_struct, NULL);
register_callback (plugin_name, PLUGIN_FINISH_UNIT,
handle_end_of_compilation_unit, NULL);
...
}
In the function body of register_callback, GCC can then create the
plugin_registration objects and chain them together based on the event
type. Because GCC will need to maintain a list of plugin_registration
objects for each event anyway, we might as well let it create (and
destroy if necessary) the objects instead of leaving the task to the
plugins.
Note that in my example an additional parameter, plugin_name, is added
in register_callback. We found it useful to have the name around when
emitting error messages if something goes wrong during the callback
registration process.
>>
>> -fplugin=/path/to/plugin.so;arg1=value;arg2=value;...
>>
I am not sure if it is GCC's responsibility to understand key-value
(or any other types of) arguments to plugins. I think GCC should
simply take a string (which, of course, can be something like
"arg1=value arg2=value") and pass it (unparsed) to the plugin. It is
plugin's job to parse/process the given arguments (whatever way it
likes). So the prototype of the plugin_init would look like this:
void plugin_init (const char *plugin_name, const char *plugin_arg);
In our current prototype, we implemented the originally proposed flag
"-fplugin-arg=", which is associated with the plugin specified in the
most recently parsed "-fplugin" flag. If a "-fplugin-arg" flag is used
in the command-line without any preceding "-fplugin", an error message
is emitted. Having the plugin name and its arguments concatenated as
currently proposed is also fine, but I would prefer a simple string
(like below) to a series of key-value pairs.
-fplugin=/path/to/plugin.so;"arguments"
(Note that the double quotes may not needed if the "arguments" doesn't
contain characters such as spaces.)
>>
>> Pass Manager
>>
We think it would be quite daunting (and probably open up a can of
worms) to allow plugins to re-order passes. So to get things moving
quickly, in our initial prototype we only support insertion of a new
pass and replacing an existing pass. When registering a new pass with
GCC, the plugin uses the normal register_callback call with the
PLUGIN_PASS_MANAGER_SETUP event. However, instead of registering a
callback function, it passes in a plugin_pass object (see below) that
specifies the opt_pass object of the new pass, the name of the
reference pass, the static instance number of the reference pass, and
whether to insert before/after or replacing the reference pass.
enum pass_positioning_ops
{
PASS_POS_INSERT_AFTER,
PASS_POS_INSERT_BEFORE,
PASS_POS_REPLACE
};
struct plugin_pass
{
struct opt_pass *pass;
const char *reference_pass_name;
/* Insert the pass at the specified instance of the reference pass.
If it's 0, do that for every instance. */
int ref_pass_instance_number;
enum pass_positioning_ops pos_op;
};
/* In plugin code */
void
plugin_init()
{
...
register_callback (plugin_name, PLUGIN_PASS_MANAGER_SETUP, NULL, &pass_info);
...
}
When registering and positioning a new pass, GCC will search the
passes chained up in all_lowering_passes, all_ipa_passes, and
all_passes (similar to Mozilla dehydra's implementation) to find the
reference pass(es) with the matching name and instance number, and
then either insert the new pass or replace the reference pass.
One caveat with our current implementation is that because the
registration of new plugin passes happens after the command-line
options are parsed, we cannot specify single pass dumping for plugin
passes (e.g. -fdump-tree-newpass). IR dumping of plugin passes is
enabled only when the dump-all flags (e.g. -fdump-tree-all) are
specified.
What do people think about this pass regi
Re: Plugin API Comments (was Re: GCC Plug-in Framework ready to port)
Attached please find the patch of our initial prototype of GCC plugin
support based on the APIs described in the (old) wiki page. I also
attached a sample plugin program (dumb-example.c) that shows how a
plugin uses the APIs.
Sean and Taras (and others),
Diego will be creating a branch for the plugin support soon. I know we
still have some issues that have yet to converge (such as flags for
plugin arguments). But in order to get things moving, what do you
think if we start with this patch and move from there? Or if you have
other patches available that implement the currently proposed APIs,
we can start with those too. Thanks.
Le-chun
On Thu, Feb 5, 2009 at 3:27 PM, Taras Glek wrote:
> Le-Chun Wu wrote:
>>
>> Hi Sean,
>>
>> It's great that you updated the wiki page with the latest and more
>> detailed API design.
>>
>> We (at Google) also started to look at the GCC plugin support a couple
>> of weeks ago. We had a quick prototype implemented based on the
>> original APIs that Taras put together in the old wiki. (I can send out
>> the patch later for people's reference.) The updated APIs in general
>> look good to me. I do have some comments based on our experience with
>> the initial prototyping:
>>
>
> Neat! I'd love to see that.
>>
>>
>>>>
>>>> void register_callbacks(int nregistrations, struct plugin_registration
>>>> *registrations);
>>>>
>>>>
>>
>> Instead of passing in an array of plugin_registration objects with a
>> single register_callbacks call, it's probably better to have the
>> plugin call a sequence of register_callback with the necessary
>> information, as shown in the following example:
>>
>> void plugin_register_callback (const char *plugin_name, enum
>> plugin_event event, plugin_callback_func callback, void *user_data);
>>
>> /* In plugin code */
>> void
>> plugin_init()
>> {
>> ...
>> register_callback (plugin_name, PLUGIN_FINISH_STRUCT, handle_struct,
>> NULL);
>> register_callback (plugin_name, PLUGIN_FINISH_UNIT,
>> handle_end_of_compilation_unit, NULL);
>> ...
>> }
>>
>> In the function body of register_callback, GCC can then create the
>> plugin_registration objects and chain them together based on the event
>> type. Because GCC will need to maintain a list of plugin_registration
>> objects for each event anyway, we might as well let it create (and
>> destroy if necessary) the objects instead of leaving the task to the
>> plugins.
>>
>> Note that in my example an additional parameter, plugin_name, is added
>> in register_callback. We found it useful to have the name around when
>> emitting error messages if something goes wrong during the callback
>> registration process.
>>
>
> Sean,
> I agree with Le-Chun that separate register_callback calls would be better.
> Would you change that(in the interest of not having too many people modify
> the api), or should I? By the way, reformatting made the page much more
> readable, thanks.
>
> I think having a plugin-name parameter is something we can decide on later (
> just like the version stuff). I can see it being useful for debugging a
> situation with multiple loaded plugins, but I'm not convinced that it will
> be a common problem.
>
>>
>>>>
>>>> -fplugin=/path/to/plugin.so;arg1=value;arg2=value;...
>>>>
>>>>
>>
>> I am not sure if it is GCC's responsibility to understand key-value
>> (or any other types of) arguments to plugins. I think GCC should
>> simply take a string (which, of course, can be something like
>> "arg1=value arg2=value") and pass it (unparsed) to the plugin. It is
>> plugin's job to parse/process the given arguments (whatever way it
>> likes). So the prototype of the plugin_init would look like this:
>>
>> void plugin_init (const char *plugin_name, const char *plugin_arg);
>>
>> In our current prototype, we implemented the originally proposed flag
>> "-fplugin-arg=", which is associated with the plugin specified in the
>> most recently parsed "-fplugin" flag. If a "-fplugin-arg" flag is used
>> in the command-line without any preceding "-fplugin", an error message
>> is emitted. Having the plugin name and its arguments concatenated as
>> currently proposed is also fine, but I would prefer a simple string
>> (like below) to a series of key-value pairs.
>>
>> -fplugin=/path/to/plugin.so;"arguments"
>>
>> (Note that the double quotes may not
Re: Plugin API Comments (was Re: GCC Plug-in Framework ready to port)
Sean, I don't really think most people are contributing to GCC for the bragging rights (at least I am not :-)). I would consider this a concerted, collaborating effort to get a simple, robust, and accommodating plugin design and implementation out as quickly as possible so that people can start using it. We offered to start with our patch simply because we had an implementation of the originally proposed APIs and we wanted to get the ball rolling quickly. As I mentioned in my previous email, if you (or other people/groups) have an implementation of the proposed APIs and prefer to start with yours (or theirs), I am all for it. (And please send the patch out so that people can look at it. Thanks.) Based on our initial experience, it's not that huge a task to implement basic GCC infrastructure supports for plugins. In fact, all the tasks you listed have been implemented in our patch (or in any other implementations, I believe), although there are certainly some corner cases that we didn't consider and some issues that have yet to converge. I think we just need to get started with an implementation of the proposed APIs (of course after reviewing and everything), and people can start iterating on that and adding new features/enhancements. (It's actually great that many of us have experiences implementing the plugin supports so that collectively we can cover more corner cases.) (BTW, a colleague pointed out to me that with my name in the header comment of the source files in our patch, people might have an impression that I am taking all the credits. That's certainly not my intention. I simply copied it from a standard header template that I kept for my GCC development. If we decide to start with that patch, I will certainly take it out.) Regards, Le-chun On Fri, Feb 6, 2009 at 2:54 AM, Sean Callanan wrote: > As a matter of protocol, I know there are several groups that all have > implementations. I bet any one of them would love to have the credit of > having their implementation be the one that got adopted. (I know ours > would! We're academics and would love to claim bragging rights.) > > In practice, Diego said that we have "several months" before we need to > integrate. Diego has graciously agreed to review patches; how about instead > of having a single implementation that goes in, we all pool our development > resources and do different portions of a fresh implementation? We could put > up a list of tasks on the Wiki and assign them to groups, so that everybody > has a hand in it. > > A possible list of tasks (which could each have a patch to itself) is: > > - Modify the GCC link process to use libltdl and libtool -export-dynamic > - Add the custom argument handler > - Implement the callback registration code > - Add hooks at relevant locations in GCC > - Implement the plug-in loading code and plug-in initialization > > Sean > > On Feb 5, 2009, at 7:14 PM, Le-Chun Wu wrote: > >> Attached please find the patch of our initial prototype of GCC plugin >> support based on the APIs described in the (old) wiki page. I also >> attached a sample plugin program (dumb-example.c) that shows how a >> plugin uses the APIs. >> >> Sean and Taras (and others), >> >> Diego will be creating a branch for the plugin support soon. I know we >> still have some issues that have yet to converge (such as flags for >> plugin arguments). But in order to get things moving, what do you >> think if we start with this patch and move from there? Or if you have >> other patches available that implement the currently proposed APIs, >> we can start with those too. Thanks. >> >> Le-chun > >
Re: [plugins] Comparison of plugin mechanisms
Hi Grigori and Zbigniew, I just took a look at the wiki page you guys put together. While I have some reservations about categorizing the plugin APIs into "production", "research", and "new pass", I totally agreed with you that the plugin support should be implemented in layers, starting from the core support that provides basic APIs and exposes all the internal GCC data structures, to more complicated support that allows more abstract APIs and maybe drastic changes in GCC compilation pipeline. I agree with Basile that the support for pass management should not be in a different category. I think allowing plugin modules to hook new passes into GCC is an important requirement for plugins to be really useful, so we should provide basic pass management support (such as adding a new pass or replacing an existing pass) even in the core (or "production") APIs. And in the advanced/extended (or "research") APIs, we can allow the whole pass manager to be replaced (probably like what you did in ICI). In our plugin prototype, we have implemented basic support for adding new passes and defined APIs that allow the plugin writers to specify where/when to hook in the new passes. I will modify the GCC Plugin API wiki with our proposal so that people can comment on it. You mentioned that you will be sending out a patch this week. Will your patch contains the implementation for both the "production" and "research" APIs? If your patch contains only the research APIs, we can prepare an official patch for the "production" APIs (based on the patch that I send out a while back ago) and send it out for review this week (if no one else would like to do it, that is). Thanks, Le-chun On Fri, Feb 13, 2009 at 1:26 AM, Grigori Fursin wrote: > Hi Basile et al, > > Thanks a lot for detailed explanations. > > In addition to Zbigniew's reply: > I didn't specifically put MELT into new category (I actually just > updated the page and added it to the other categories too), > but I still keep the new pass integration plugin, since > some of our colleagues would like to add new passes including > new optimizations, but have difficulties to do that since there > is no full documented API, they may not want to write it in C > (as you mentioned) and they are concerned about support overheads > if the internal API will be changing all the time. Providing > standard API would be of great help for such cases, but it rises > other issues such as opening GCC to proprietary plugins, etc > which has been heavily discussed here for some time. So I personally > think we should leave it for the future and just implement minimalistic > Plugin API that will already make many current and prospective users > happy and then we can gradually extend it ... > > By the way, Sean, what do you think about the proposed APIs? > Will it suit your purposes or some changes are also required? > I would like to add info about your plugin system but just > don't have much knowledge about that ... > > Cheers, > Grigori > >> -Original Message- >> From: Basile STARYNKEVITCH [mailto:bas...@starynkevitch.net] >> Sent: Friday, February 13, 2009 8:38 AM >> To: Grigori Fursin >> Cc: 'Diego Novillo'; gcc@gcc.gnu.org; 'Sean Callanan'; 'Taras Glek'; >> 'Le-Chun Wu'; 'Gerald >> Pfeifer'; 'Zbigniew Chamski'; 'Cupertino Miranda' >> Subject: Re: [plugins] Comparison of plugin mechanisms >> >> Hello All, >> >> >> Grigori Fursin wrote: >> > Basically, we currently see 3 complementary categories of GCC plugins, >> > depending >> > on the nature of the extension: production, experimentation/research, and >> > new pass >> > integration. Each category naturally calls for slightly different API >> > features. >> > >> > >> I am not sure of the relevance of the "new pass integration plugins" >> examplified by MELT. >> [on the other hand, I do know Grigori and I believe he thought quite a >> lot about plugins, which I didn't. I only implemented one particular >> plugin machinery = MELT, knowing well that my approach is quite peculiar >> both in its goals and its implementation. I never thought of MELT as a >> universal plugin machinery). >> >> In my view, MELT fits quite precisely in the "production plugins" >> definition, while indeed I expect it to be useful mostly for >> "experimental/research" plugins. >> >> In my view also, the "new pass integration plugin" category should not >> really exist, because it probably can fit inside one (or both) of the >>
Thread safety annotations and analysis in GCC
Hi, We have been working on creating program annotations for C/C++ (in GCC attributes) to help developers document locks and how they need to be used to safely read and write shared variables in multi-threaded code. We've also implemented a new GCC pass that uses the annotations to identify and warn about the issues that could potentially result in race conditions and deadlocks. Here is the design doc for the proposed annotations: http://docs.google.com/Doc?id=ddqtfwhb_0c49t6zgr Attached please find also find the patch file that contains the initial implementation of the annotations and analysis. Please let me know if you have any feedback/comments/questions on the proposed annotations and the GCC implementation. We will be creating a public branch for people who are interested to start contributing to the project (before the patch is accepted and checked into the trunk). We will send out another email once the public branch is created. Thanks, Le-Chun Wu ts_annotations.patch.gz Description: GNU Zip compressed data
Re: Thread safety annotations and analysis in GCC
You are right that our current proposal/implementation doesn't handle
indirect function calls. (And we didn't try to do any pointer/alias
analysis for variables either in our first implementation .) But the
analysis will not be disabled completely if it encounters an indirect
function call. It simply ignores the indirect call. For example, when
analyzing function "foo" in the following code,
Mutex mu1, mu2;
int g GUARDED_BY(mu1);
void bar() EXCLUSIVE_LOCKS_REQUIRED(mu2);
int foo(int x, void (*indirect_func)())
{
mu1.Lock();
indirect_func();
g = x + 2;
mu1.Unlock();
}
main()
{
foo(3, bar);
...
}
the analyzer will not be able to check the lock requirements for
"indirect_func", but it will continue to check whether writing to
variable "g" is indeed guarded by "mu1".
I think we tried to make the initial version of the
annotations/analysis simple so that users won't get too intimidated by
a very complex annotation system that covers all the cases. But
certainly it will be extended in the future. (Supporting the function
pointers/indirect calls is certainly one of the enhancements to
consider.)
Thanks,
Le-chun
On Mon, Jun 9, 2008 at 8:33 PM, Andi Kleen <[EMAIL PROTECTED]> wrote:
> "Le-Chun Wu" <[EMAIL PROTECTED]> writes:
>>
>> Please let me know if you have any feedback/comments/questions on the
>> proposed annotations and the GCC implementation.
>
> I was surprised that there was no consideration of C indirect function
> calls in your design document. e.g. I would have expect some way
> to give a hint that a indirect function call is always called from X
> to propagate the lock sets. Right now it looks like they will
> disable the analysis completely, correct?
>
> -Andi
>
>
testsuite location for adding new tests
Hi, As part of our thread safety annotation/analysis effort, we created about 17 new test cases that we would like to add to the gcc testsuite. Should we create a new sub-directory under testsuite/g++.dg (say, for example, g++.dg/thread-ann)? Or should we add them to an existing sub-dir, e.g. testsuite/g++.dg/warn? (Some of the test cases would cause the compiler to emit thread safety warning messages, but some of them are "good" cases that no warning should be generated. So we are not sure if they should all go to g++.dg/warn.) Thanks, Le-chun
Re: Thread safety annotations and analysis in GCC
Tom, Thanks a lot for pointing us to the sparse annotations. We will take a look and see what its support looks like. Le-chun On Sun, Jun 15, 2008 at 2:43 PM, Tom Tromey <[EMAIL PROTECTED]> wrote: >>>>>> "Le-Chun" == Le-Chun Wu <[EMAIL PROTECTED]> writes: > > Le-Chun> Here is the design doc for the proposed annotations: > Le-Chun> http://docs.google.com/Doc?id=ddqtfwhb_0c49t6zgr > > I am curious to know how this compares to the kind of lock checking > implemented in sparse, and in particular whether sparse annotations > could easily be translated to this style. > > Tom >
thread-annotations branch created
Hi, I have created the branch "thread-annotations" that contains the implementation of the thread safety annotations and analysis (described in http://docs.google.com/Doc?id=ddqtfwhb_0c49t6zgr). I have committed the initial implementation and a set of new test cases to the branch. I've also created a wiki page for the project at http://gcc.gnu.org/wiki/ThreadSafetyAnnotation Thanks, Le-chun
How to determine if a decl is a class member in GCC
Hi,
In my attribute handlers that handle the new thread-safety attributes
(in c-common.c), I need to check if a decl is a class member. How do
people do that? My original code was checking if a decl is a
FIELD_DECL but that doesn't work for static members. I also tried to
use DECL_CONTEXT but it is not set (in the C++ front-end) for data
members. (I was able to use DECL_CONTEXT for member functions,
though.) Is there any other way that I should use?
BTW, as an experiment, I went ahead and made the following change in
C++ front-end to set the DECL_CONTEXT of class data members. The patch
appears to work and doesn't seem to break any g++ and gcc regression
tests. (I will be running gdb testsuite later.) I think there must be
a reason why the DECL_CONTEXT of data members wasn't set in the
front-end, but it's not clear to my why. Can someone kindly explain
why that is and whether the patch is OK?
Thanks,
Le-chun
Index: cp/decl.c
===
--- cp/decl.c (revision 137849)
+++ cp/decl.c (working copy)
@@ -9074,6 +9074,11 @@ grokdeclarator (const cp_declarator *dec
if (thread_p)
DECL_TLS_MODEL (decl) = decl_default_tls_model (decl);
+
+/* Since a static class data member is a VAR_DECL (instead of
+ a FIELD_DECL), setting the decl_context here allows us to
+ tell if it is a class member. */
+DECL_CONTEXT (decl) = ctype ? ctype : current_class_type;
}
else
{
Re: How to determine if a decl is a class member in GCC
Hi,
I haven't heard anything back on my questions. Can any of C++ frontend
maintainers please shed some light (or comment on my proposed patch)?
Thanks a lot.
Le-chun
On Fri, Jul 18, 2008 at 10:22 AM, Le-Chun Wu <[EMAIL PROTECTED]> wrote:
> Hi,
>
> In my attribute handlers that handle the new thread-safety attributes
> (in c-common.c), I need to check if a decl is a class member. How do
> people do that? My original code was checking if a decl is a
> FIELD_DECL but that doesn't work for static members. I also tried to
> use DECL_CONTEXT but it is not set (in the C++ front-end) for data
> members. (I was able to use DECL_CONTEXT for member functions,
> though.) Is there any other way that I should use?
>
> BTW, as an experiment, I went ahead and made the following change in
> C++ front-end to set the DECL_CONTEXT of class data members. The patch
> appears to work and doesn't seem to break any g++ and gcc regression
> tests. (I will be running gdb testsuite later.) I think there must be
> a reason why the DECL_CONTEXT of data members wasn't set in the
> front-end, but it's not clear to my why. Can someone kindly explain
> why that is and whether the patch is OK?
>
> Thanks,
>
> Le-chun
>
>
> Index: cp/decl.c
> ===
> --- cp/decl.c (revision 137849)
> +++ cp/decl.c (working copy)
> @@ -9074,6 +9074,11 @@ grokdeclarator (const cp_declarator *dec
>
> if (thread_p)
> DECL_TLS_MODEL (decl) = decl_default_tls_model (decl);
> +
> +/* Since a static class data member is a VAR_DECL (instead of
> + a FIELD_DECL), setting the decl_context here allows us to
> + tell if it is a class member. */
> +DECL_CONTEXT (decl) = ctype ? ctype : current_class_type;
> }
> else
> {
>
Re: How to determine if a decl is a class member in GCC
Benjamin,
Thanks for looking into this issue. I see what's going on here. It's
basically a phase ordering problem. I am trying to determine whether a
declaration is a class member when attributes are parsed and handled
(in c-common.c), which happens earlier than where the context of a
data member is set (which takes place either in
finish_static_data_member_decl or finish_member_declaration). Do you
know of any way to determine whether a decl is a class member when we
are parsing the decl attributes?
Thanks,
Le-chun
On Tue, Jul 22, 2008 at 12:34 PM, Benjamin Smedberg
<[EMAIL PROTECTED]> wrote:
> Le-Chun Wu wrote:
>>
>> Hi,
>>
>> I haven't heard anything back on my questions. Can any of C++ frontend
>> maintainers please shed some light (or comment on my proposed patch)?
>> Thanks a lot.
>>
>> Le-chun
>>
>> On Fri, Jul 18, 2008 at 10:22 AM, Le-Chun Wu <[EMAIL PROTECTED]> wrote:
>>>
>>> Hi,
>>>
>>> In my attribute handlers that handle the new thread-safety attributes
>>> (in c-common.c), I need to check if a decl is a class member. How do
>>> people do that? My original code was checking if a decl is a
>>> FIELD_DECL but that doesn't work for static members. I also tried to
>>> use DECL_CONTEXT but it is not set (in the C++ front-end) for data
>>> members. (I was able to use DECL_CONTEXT for member functions,
>>> though.) Is there any other way that I should use?
>
> I created a simple testcase for treehydra and it seems that DECL_CONTEXT was
> set and correct for all of the following in 4.3.0:
>
> struct A {
> int a;
> int f();
> static int sa;
> static int sf();
> };
>
> Are you saying that you don't see a proper DECL_CONTEXT for "a" and "sa"?
>
> --BDS
>
Re: Thread safety annotations and analysis in GCC
Ken,
Thanks a lot for your feedback. Somehow your email fell through the
cracks and I didn't notice it until a colleague of mine reminded me.
Sorry for my delay in replying.
Here are my answers to your questions.
>
> All the examples seem to be C++ oriented; is it, in fact, a goal for the
> annotations and analysis to be just as useful in C?
It's true that we started out with a design more C++ oriented, but it
is also our goal to make the annotations and analysis useful in C. For
example, the original design of the annotations for lock/unlock
primitives didn't take any argument (as we assumed the primitives
would be member functions of a lockable class), but later we changed
the design to allow the annotations to take lock arguments for C-style
lock/unlock primitives.
>
> What are the scoping rules used for finding the mutex referenced in the
> GUARDED_BY macro within a C++ class definition? Are they the same as for
> looking up identifiers in other contexts? How is the lookup done for C
> structures?
The scoping rules for annotations on class members are the same as for
looking up identifiers in, say member functions. We modified GCC
front-end to process the thread-safety attributes after the class
definition is finished parsing so that we can reference in the
annotations data members declared later in the class. For example,
referencing "mu" in the GUARDED_BY annotation in the following example
is allowed:
class Foo {
int a GUARDED_BY(mu);
Mutex mu;
};
Unfortunately in our current implementation (on the thread-annotations
branch) we haven't modified the C front-end to support this. We will
do that soon.
>
> Will the compiler get built-in knowledge of the OS library routines (e.g.,
> pthread_mutex_lock) on various platforms?
Currently no. Users will need to provide declarations with the
annotations for these library routines in their code. e.g. they will
need to provide a declaration for pthread_mutex_lock like the
following:
int pthread_mutex_lock(pthread_mutex_t *mutex) EXCLUSIVE_LOCK_FUNCTION(1);
>
> You list separate annotations for "trylock" functions. It appears that the
> difference is that "trylock" functions can fail. However,
> pthread_mutex_lock can fail, if the mutex isn't properly initialized, if
> recursive locking of a non-recursive mutex is detected, or other reasons;
> the difference between pthread_mutex_lock and pthread_mutex_trylock is
> whether it will wait or immediately return EBUSY for a mutex locked by
> another thread. So I think pthread_mutex_lock should be described as a
> "trylock" function too, under your semantics. Conservatively written code
> will check for errors, and will have a path in which the lock is assumed
> *not* to have been acquired; if the analysis assumes pthread_mutex_lock
> always succeeds, that path may be analyzed incorrectly. (I ran into a tool
> once before that complained about my locking code until I added an unlock
> call to the error handling path. Since it's actively encouraging writing
> incorrect code, I'm not using it any more.)
>
Good point. pthread_mutex_lock should be annotated as a trylock
routine so that the analyzer won't emit (bogus) warning messages in
the code shown below (which is similar to what you referred to).
if (! pthread_mutex_lock(mu))
{
/* Do something */
pthread_mutex_unlock(mu);
}
else
{
/* Error handling */
/* No need to unlock */
}
(It also means that the declaration example I gave above for
pthread_mutex_lock should be changed. :-))
Thanks again for your feedback. Please let me know if you have any
other comments/questions (or bugs if you start to play with the
analysis pass).
Le-chun
