Skip to content

[Fix bug] fix thread init bug #4891

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

Merged
merged 1 commit into from
Feb 16, 2022

Conversation

Sliverguo
Copy link
Contributor

When we create thread A by rt_thread_init but do
not init the thread object,and then create thread
B in the thread A,it maybe crash.

拉取/合并请求描述:(PR description)

[

一、问题描述

在RT_USING_MODULE特性开启的情况下,利用rt_thread_init去创建线程A,再在A线程中去创建线程B,会出现data abort的问题。

二、问题分析

1.在利用rt_thread_init去创建线程A的时候,由于第一个参数(struct rt_thread *)是由用户创建,这个rt_thread中的数据可能是随机的,也就导致其中的成员module_id的值也可能是随机的,而在创建线程B的时候,无论是利用rt_thread_init还是rt_thread_create都会调用rt_object_init函数,我现在看一下这个函数的实现。

void rt_object_init(struct rt_object         *object,
                    enum rt_object_class_type type,
                    const char               *name)
{
    register rt_base_t temp;
    struct rt_object_information *information;
#ifdef RT_USING_MODULE
    struct rt_dlmodule *module = dlmodule_self();
#endif

    /* get object information */
    information = rt_object_get_information(type);
    RT_ASSERT(information != RT_NULL);

    /* initialize object's parameters */

    /* set object type to static */
    object->type = type | RT_Object_Class_Static;

    /* copy name */
    rt_strncpy(object->name, name, RT_NAME_MAX);

    RT_OBJECT_HOOK_CALL(rt_object_attach_hook, (object));

    /* lock interrupt */
    temp = rt_hw_interrupt_disable();

#ifdef RT_USING_MODULE
    if (module)
    {
        rt_list_insert_after(&(module->object_list), &(object->list));
        object->module_id = (void *)module;
    }
    else
#endif
    {
        /* insert object into information object list */
        rt_list_insert_after(&(information->object_list), &(object->list));
    }

    /* unlock interrupt */
    rt_hw_interrupt_enable(temp);
}
  1. 从上面代码可以看到,如果线程A的module_id未被初始化0的情况下会进入到if module分支,会引用此时的module,此时module是随机,就会出现data abort的情况。

三、解决方法

从上面的分析可知,导致此问题的原因主要是用户创建的struct rt_thread不可预测,所以应该在rt_thread_init处进行memset。如下图所示:

image

此种解决方法已在柿饼M3上验证。

四、疑问

1.为何用rt_thread_create时未出现?

因为在用此函数时,会进行memset。

2.如果用rt_thread_init不进行初始化atruct rt_thread结构体,是否会引起其它问题?

存在这种可能,但是如果全部memset则应该会避免。
]

以下的内容不应该在提交PR时的message修改,修改下述message,PR会被直接关闭。请在提交PR后,浏览器查看PR并对以下检查项逐项check,没问题后逐条在页面上打钩。
The following content must not be changed in the submitted PR message. Otherwise, the PR will be closed immediately. After submitted PR, please use a web browser to visit PR, and check items one by one, and ticked them if no problem.

当前拉取/合并请求的状态 Intent for your PR

必须选择一项 Choose one (Mandatory):

  • 本拉取/合并请求是一个草稿版本 This PR is for a code-review and is intended to get feedback
  • 本拉取/合并请求是一个成熟版本 This PR is mature, and ready to be integrated into the repo

代码质量 Code Quality:

我在这个拉取/合并请求中已经考虑了 As part of this pull request, I've considered the following:

  • 已经仔细查看过代码改动的对比 Already check the difference between PR and old code
  • 代码风格正确,包括缩进空格,命名及其他风格 Style guide is adhered to, including spacing, naming and other styles
  • 没有垃圾代码,代码尽量精简,不包含#if 0代码,不包含已经被注释了的代码 All redundant code is removed and cleaned up
  • 所有变更均有原因及合理的,并且不会影响到其他软件组件代码或BSP All modifications are justified and not affect other components or BSP
  • 对难懂代码均提供对应的注释 I've commented appropriately where code is tricky
  • 本拉取/合并请求代码是高质量的 Code in this PR is of high quality
  • 本拉取/合并符合RT-Thread代码规范 This PR complies with RT-Thread code specification

@CLAassistant
Copy link

CLAassistant commented Jul 27, 2021

CLA assistant check
All committers have signed the CLA.

@BernardXiong
Copy link
Member

这个问题犯难的了,如果在调用rt_thread_init之前,上层应用已经对tcb做了部分赋值,那么这里memset会全部清零。

BTW, 代码需要遵循RT-Thread的风格

@Sliverguo
Copy link
Contributor Author

个人认为,TCB部分本来就是内核需要维护的数据,用户数据不应该直接赋值到TCB中,应该通过参数的形式传入。用户只是给TCB一个内存空间,而不应该关心TCB的具体内容,或者TCB的内存空间也不需要用户分配,那么这种情况下rt_thread_init就没有存在的意义了,所以还是觉得全部清0比较靠谱。

@Sliverguo Sliverguo force-pushed the fix_rt_rhtread_init_bug branch from 0bfdc9b to bcc2863 Compare August 4, 2021 07:57
@mysterywolf
Copy link
Member

mysterywolf commented Dec 30, 2021

这个问题犯难的了,如果在调用rt_thread_init之前,上层应用已经对tcb做了部分赋值,那么这里memset会全部清零。

可否手动对未赋值的成员变量逐一赋值? 含着垃圾直确实不太好。

@mysterywolf mysterywolf added urgent 🏃 urgent proposal proposal for future version and removed urgent 🏃 urgent labels Jan 12, 2022
@BernardXiong
Copy link
Member

BTW, 代码请遵循RT-Thread的风格

@mysterywolf mysterywolf mentioned this pull request Jan 29, 2022
9 tasks
@mysterywolf mysterywolf reopened this Jan 30, 2022
@mysterywolf
Copy link
Member

请参照 #5580 进行修改

@mysterywolf
Copy link
Member

新年好 楼主大哥在吗

@mysterywolf
Copy link
Member

@Sliverguo 元宵节快乐 楼主大哥在吗

@Sliverguo
Copy link
Contributor Author

在的,具体的修改方向我还没明白,能具体讲一下吗?

@mysterywolf
Copy link
Member

mysterywolf commented Feb 16, 2022

在的,具体的修改方向我还没明白,能具体讲一下吗?

你就把我这个pr #5580 抄过去就可以了,或者你直接授权我来改。

    When we create thread A by rt_thread_init but do
    not init the thread object,and then create thread
    B in the thread A,it maybe crash.
@Sliverguo Sliverguo force-pushed the fix_rt_rhtread_init_bug branch from bcc2863 to 10a6827 Compare February 16, 2022 03:16
@mysterywolf mysterywolf added +1 Agree +1 important and removed proposal proposal for future version labels Feb 16, 2022
@Guozhanxin Guozhanxin added the +2 Agree +2 label Feb 16, 2022
@BernardXiong BernardXiong merged commit 01b3a34 into RT-Thread:master Feb 16, 2022
QIANWC pushed a commit to QIANWC/rt-thread that referenced this pull request Feb 24, 2022
When we create thread A by rt_thread_init but do
    not init the thread object,and then create thread
    B in the thread A,it maybe crash.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
important +1 Agree +1 +2 Agree +2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants